[hibernate-commits] Hibernate SVN: r20156 - in core/trunk: testsuite/src/test/java/org/hibernate/test/cascade and 1 other directories.

hibernate-commits at lists.jboss.org hibernate-commits at lists.jboss.org
Tue Aug 17 14:22:38 EDT 2010


Author: gbadner
Date: 2010-08-17 14:22:38 -0400 (Tue, 17 Aug 2010)
New Revision: 20156

Added:
   core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeCheckNullibilityFalseTest.java
   core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeCheckNullibilityTrueTest.java
Modified:
   core/trunk/core/src/main/java/org/hibernate/event/def/DefaultMergeEventListener.java
   core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/MultiPathCascadeTest.java
   core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/CascadeMergeToChildBeforeParentTest.java
   core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeTest.java
   core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/Node.java
Log:
HHH-5473 : Default for CHECK_NULLABILITY does not allow merge retries

Modified: core/trunk/core/src/main/java/org/hibernate/event/def/DefaultMergeEventListener.java
===================================================================
--- core/trunk/core/src/main/java/org/hibernate/event/def/DefaultMergeEventListener.java	2010-08-17 15:12:41 UTC (rev 20155)
+++ core/trunk/core/src/main/java/org/hibernate/event/def/DefaultMergeEventListener.java	2010-08-17 18:22:38 UTC (rev 20156)
@@ -89,21 +89,28 @@
 		// For now, just retry once; throw TransientObjectException if there are still any transient entities
 		Map transientCopyCache = getTransientCopyCache(event, copyCache );
 		if ( transientCopyCache.size() > 0 ) {
-			retryMergeTransientEntities( event, transientCopyCache, copyCache );
+			retryMergeTransientEntities( event, transientCopyCache, copyCache, true );
 			// find any entities that are still transient after retry
 			transientCopyCache = getTransientCopyCache(event, copyCache );
 			if ( transientCopyCache.size() > 0 ) {
 				Set transientEntityNames = new HashSet();
-				for( Iterator it=transientCopyCache.keySet().iterator(); it.hasNext(); ) {
-					Object transientEntity = it.next();
+				for( Iterator it=transientCopyCache.entrySet().iterator(); it.hasNext(); ) {
+					Object transientEntity = ( ( Map.Entry ) it.next() ).getKey();
 					String transientEntityName = event.getSession().guessEntityName( transientEntity );
 					transientEntityNames.add( transientEntityName );
-					log.trace( "transient instance could not be processed by merge: " +
+					log.trace( "transient instance could not be processed by merge when checking nullability: " +
 							transientEntityName + "[" + transientEntity + "]" );
 				}
-				throw new TransientObjectException(
-					"one or more objects is an unsaved transient instance - save transient instance(s) before merging: " +
-					transientEntityNames );
+				if ( isNullabilityCheckedGlobal( event.getSession() ) ) {
+					throw new TransientObjectException(
+						"one or more objects is an unsaved transient instance - save transient instance(s) before merging: " +
+						transientEntityNames );
+				}
+				else {
+					log.trace( "retry saving transient instances without checking nullability" );
+					// failures will be detected later...
+					retryMergeTransientEntities( event, transientCopyCache, copyCache, false );
+				}
 			}
 		}
 		copyCache.clear();
@@ -125,10 +132,18 @@
 				// TODO: cache the entity name somewhere so that it is available to this exception
 				log.trace( "transient instance could not be processed by merge: " +
 						event.getSession().guessEntityName( copy ) + "[" + entity + "]" );
-				throw new TransientObjectException(
-					"object is an unsaved transient instance - save the transient instance before merging: " +
-						event.getSession().guessEntityName( copy )
-				);
+				// merge did not cascade to this entity; it's in copyCache because a
+				// different entity has a non-nullable reference to this entity;
+				// this entity should not be put in transientCopyCache, because it was
+				// not included in the merge;
+				// if the global setting for checking nullability is false, the non-nullable
+				// reference to this entity will be detected later
+				if ( isNullabilityCheckedGlobal( event.getSession() ) ) {
+					throw new TransientObjectException(
+						"object is an unsaved transient instance - save the transient instance before merging: " +
+							event.getSession().guessEntityName( copy )
+					);
+				}
 			}
 			else if ( copyEntry.getStatus() == Status.SAVING ) {
 				transientCopyCache.put( entity, copy, copyCache.isOperatedOn( entity ) );
@@ -140,7 +155,11 @@
 		return transientCopyCache;
 	}
 
-	protected void retryMergeTransientEntities(MergeEvent event, Map transientCopyCache, EventCache copyCache) {
+	protected void retryMergeTransientEntities(
+			MergeEvent event,
+			Map transientCopyCache,
+			EventCache copyCache,
+			boolean isNullabilityChecked) {
 		// TODO: The order in which entities are saved may matter (e.g., a particular transient entity
 		//       may need to be saved before other transient entities can be saved;
 		//       Keep retrying the batch of transient entities until either:
@@ -152,12 +171,14 @@
 			Object entity = mapEntry.getKey();
 			Object copy = transientCopyCache.get( entity );
 			EntityEntry copyEntry = event.getSession().getPersistenceContext().getEntry( copy );
-			if ( entity == event.getEntity() ) {
-				mergeTransientEntity( entity, copyEntry.getEntityName(), event.getRequestedId(), event.getSession(), copyCache);
-			}
-			else {
-				mergeTransientEntity( entity, copyEntry.getEntityName(), copyEntry.getId(), event.getSession(), copyCache);
-			}
+			mergeTransientEntity(
+					entity,
+					copyEntry.getEntityName(),
+					( entity == event.getEntity() ? event.getRequestedId() : copyEntry.getId() ),
+					event.getSession(),
+					copyCache,
+					isNullabilityChecked
+			);
 		}
 	}
 
@@ -279,11 +300,21 @@
 		final EntityPersister persister = source.getEntityPersister( event.getEntityName(), entity );
 		final String entityName = persister.getEntityName();
 
-		event.setResult( mergeTransientEntity( entity, entityName, event.getRequestedId(), source, copyCache ) );
+		event.setResult( mergeTransientEntity( entity, entityName, event.getRequestedId(), source, copyCache, true ) );
 	}
 
 	protected Object mergeTransientEntity(Object entity, String entityName, Serializable requestedId, EventSource source, Map copyCache) {
+		return mergeTransientEntity( entity, entityName, requestedId, source, copyCache, true );
+	}
 
+	private Object mergeTransientEntity(
+			Object entity,
+			String entityName,
+			Serializable requestedId,
+			EventSource source,
+			Map copyCache,
+			boolean isNullabilityChecked) {
+
 		log.trace("merging transient instance");
 
 		final EntityPersister persister = source.getEntityPersister( entityName, entity );
@@ -306,15 +337,8 @@
 		copyValues(persister, entity, copy, source, copyCache, ForeignKeyDirection.FOREIGN_KEY_FROM_PARENT);
 
 		try {
-			//this bit is only *really* absolutely necessary for handling
-			//requestedId, but is also good if we merge multiple object
-			//graphs, since it helps ensure uniqueness
-			if (requestedId==null) {
-				saveWithGeneratedId( copy, entityName, copyCache, source, false );
-			}
-			else {
-				saveWithRequestedId( copy, requestedId, entityName, copyCache, source );
-			}
+			// try saving; check for non-nullable properties that are null or transient entities before saving
+			saveTransientEntity( copy, entityName, requestedId, source, copyCache, isNullabilityChecked );
 		}
 		catch (PropertyValueException ex) {
 			String propertyName = ex.getPropertyName();
@@ -325,12 +349,26 @@
 			if ( propertyFromCopy == null || ! propertyType.isEntityType() ) {
 				log.trace( "property '" + copyEntry.getEntityName() + "." + propertyName +
 						"' is null or not an entity; " + propertyName + " =["+propertyFromCopy+"]");
-				throw ex;
+				if ( isNullabilityCheckedGlobal( source ) ) {
+					throw ex;
+				}
+				else {
+					// retry save w/o checking for non-nullable properties
+					// (the failure will be detected later)
+					saveTransientEntity( copy, entityName, requestedId, source, copyCache, false );
+				}
 			}
 			if ( ! copyCache.containsKey( propertyFromEntity ) ) {
 				log.trace( "property '" + copyEntry.getEntityName() + "." + propertyName +
 						"' from original entity is not in copyCache; " + propertyName + " =["+propertyFromEntity+"]");
-				throw ex;
+				if ( isNullabilityCheckedGlobal( source ) ) {
+					throw ex;
+				}
+				else {
+					// retry save w/o checking non-nullable properties
+					// (the failure will be detected later)
+					saveTransientEntity( copy, entityName, requestedId, source, copyCache, false );					
+				}
 			}
 			if ( ( ( EventCache ) copyCache ).isOperatedOn( propertyFromEntity ) ) {
 				log.trace( "property '" + copyEntry.getEntityName() + "." + propertyName +
@@ -354,6 +392,36 @@
 
 	}
 
+	private boolean isNullabilityCheckedGlobal(EventSource source) {
+		return source.getFactory().getSettings().isCheckNullability();
+	}
+
+	private void saveTransientEntity(
+			Object entity,
+			String entityName,
+			Serializable requestedId,
+			EventSource source,
+			Map copyCache,
+			boolean isNullabilityChecked) {
+
+		boolean isNullabilityCheckedOrig =
+			source.getFactory().getSettings().isCheckNullability();
+		try {
+			source.getFactory().getSettings().setCheckNullability( isNullabilityChecked );
+			//this bit is only *really* absolutely necessary for handling
+			//requestedId, but is also good if we merge multiple object
+			//graphs, since it helps ensure uniqueness
+			if (requestedId==null) {
+				saveWithGeneratedId( entity, entityName, copyCache, source, false );
+			}
+			else {
+				saveWithRequestedId( entity, requestedId, entityName, copyCache, source );
+			}
+		}
+		finally {
+			source.getFactory().getSettings().setCheckNullability( isNullabilityCheckedOrig );
+		}
+	}
 	protected void entityIsDetached(MergeEvent event, Map copyCache) {
 		
 		log.trace("merging detached instance");

Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/MultiPathCascadeTest.java
===================================================================
--- core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/MultiPathCascadeTest.java	2010-08-17 15:12:41 UTC (rev 20155)
+++ core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/MultiPathCascadeTest.java	2010-08-17 18:22:38 UTC (rev 20156)
@@ -198,6 +198,7 @@
 		try {
 			s.merge( a );
 			s.merge( h );
+			s.getTransaction().commit();
 			fail( "should have thrown TransientObjectException" );
 		}
 		catch ( TransientObjectException ex ) {
@@ -246,6 +247,7 @@
 		try {
 			s.merge( a );
 			s.merge( g );
+			s.getTransaction().commit();
 			fail( "should have thrown TransientObjectException" );
 		}
 		catch ( TransientObjectException ex ) {
@@ -294,6 +296,7 @@
 		try {
 			s.merge( a );
 			s.merge( h );
+			s.getTransaction().commit();
 			fail( "should have thrown TransientObjectException" );
 		}
 		catch ( TransientObjectException ex ) {

Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/CascadeMergeToChildBeforeParentTest.java
===================================================================
--- core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/CascadeMergeToChildBeforeParentTest.java	2010-08-17 15:12:41 UTC (rev 20155)
+++ core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/CascadeMergeToChildBeforeParentTest.java	2010-08-17 18:22:38 UTC (rev 20156)
@@ -66,7 +66,9 @@
 		return new String[] {
 				"cascade/circle/CascadeMergeToChildBeforeParent.hbm.xml"
 		};
-	}@Override
+	}
+
+	@Override
 	 public void configure(Configuration cfg) {
 		super.configure( cfg );
 		cfg.setProperty( Environment.CHECK_NULLABILITY, "true" );

Added: core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeCheckNullibilityFalseTest.java
===================================================================
--- core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeCheckNullibilityFalseTest.java	                        (rev 0)
+++ core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeCheckNullibilityFalseTest.java	2010-08-17 18:22:38 UTC (rev 20156)
@@ -0,0 +1,51 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2010, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ *
+ */
+package org.hibernate.test.cascade.circle;
+
+import junit.framework.Test;
+
+import org.hibernate.cfg.Configuration;
+import org.hibernate.cfg.Environment;
+import org.hibernate.testing.junit.functional.FunctionalTestClassTestSuite;
+
+/**
+ * @author Gail Badner
+ */
+public class MultiPathCircleCascadeCheckNullibilityFalseTest extends MultiPathCircleCascadeTest {
+
+	public MultiPathCircleCascadeCheckNullibilityFalseTest(String str) {
+		super( str );
+	}
+
+	@Override
+	 public void configure(Configuration cfg) {
+		super.configure( cfg );
+		cfg.setProperty( Environment.CHECK_NULLABILITY, "false" );
+	}
+
+	public static Test suite() {
+		return new FunctionalTestClassTestSuite( MultiPathCircleCascadeCheckNullibilityFalseTest.class );
+	}
+}
\ No newline at end of file

Added: core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeCheckNullibilityTrueTest.java
===================================================================
--- core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeCheckNullibilityTrueTest.java	                        (rev 0)
+++ core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeCheckNullibilityTrueTest.java	2010-08-17 18:22:38 UTC (rev 20156)
@@ -0,0 +1,51 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2010, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ *
+ */
+package org.hibernate.test.cascade.circle;
+
+import junit.framework.Test;
+
+import org.hibernate.cfg.Configuration;
+import org.hibernate.cfg.Environment;
+import org.hibernate.testing.junit.functional.FunctionalTestClassTestSuite;
+
+/**
+ * @author Gail Badner
+ */
+public class MultiPathCircleCascadeCheckNullibilityTrueTest extends MultiPathCircleCascadeTest {
+
+	public MultiPathCircleCascadeCheckNullibilityTrueTest(String str) {
+		super( str );
+	}
+
+	@Override
+	 public void configure(Configuration cfg) {
+		super.configure( cfg );
+		cfg.setProperty( Environment.CHECK_NULLABILITY, "true" );
+	}
+
+	public static Test suite() {
+		return new FunctionalTestClassTestSuite( MultiPathCircleCascadeCheckNullibilityTrueTest.class );
+	}
+}
\ No newline at end of file

Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeTest.java
===================================================================
--- core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeTest.java	2010-08-17 15:12:41 UTC (rev 20155)
+++ core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/MultiPathCircleCascadeTest.java	2010-08-17 18:22:38 UTC (rev 20156)
@@ -30,9 +30,13 @@
 
 import junit.framework.Test;
 
+import org.hibernate.JDBCException;
+import org.hibernate.PropertyValueException;
 import org.hibernate.Session;
+import org.hibernate.TransientObjectException;
 import org.hibernate.cfg.Configuration;
 import org.hibernate.cfg.Environment;
+import org.hibernate.engine.SessionImplementor;
 import org.hibernate.testing.junit.functional.FunctionalTestCase;
 import org.hibernate.testing.junit.functional.FunctionalTestClassTestSuite;
 
@@ -67,7 +71,6 @@
 	public void configure(Configuration cfg) {
 		cfg.setProperty( Environment.GENERATE_STATISTICS, "true");
 		cfg.setProperty( Environment.STATEMENT_BATCH_SIZE, "0" );
-		cfg.setProperty( Environment.CHECK_NULLABILITY, "true" );
 	}
 
 	public String[] getMappings() {
@@ -89,6 +92,95 @@
 		s.createQuery( "delete from Route" );
 	}
 	
+	public void testMergeEntityWithNonNullableTransientEntity()
+	{
+		Route route = getUpdatedDetachedEntity();
+
+		Node node = ( Node ) route.getNodes().iterator().next();
+		route.getNodes().remove( node );
+
+		Route routeNew = new Route();
+		routeNew.setName( "new route" );
+		routeNew.getNodes().add( node );
+		node.setRoute( routeNew );
+
+		Session s = openSession();
+		s.beginTransaction();
+
+		try {
+			s.merge( node );
+			fail( "should have thrown an exception" );
+		}
+		catch ( Exception ex ) {
+			if ( ( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability() ) {
+				assertTrue( ex instanceof TransientObjectException );
+			}
+			else {
+				assertTrue( ex instanceof JDBCException );
+			}
+		}
+		finally {
+			s.getTransaction().rollback();
+			s.close();
+		}
+	}
+
+	public void testMergeEntityWithNonNullableEntityNull()
+	{
+		Route route = getUpdatedDetachedEntity();
+
+		Node node = ( Node ) route.getNodes().iterator().next();
+		route.getNodes().remove( node );
+		node.setRoute( null );
+
+		Session s = openSession();
+		s.beginTransaction();
+
+		try {
+			s.merge( node );
+			fail( "should have thrown an exception" );
+		}
+		catch ( Exception ex ) {
+			if ( ( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability() ) {
+				assertTrue( ex instanceof PropertyValueException );
+			}
+			else {
+				assertTrue( ex instanceof JDBCException );
+			}
+		}
+		finally {
+			s.getTransaction().rollback();
+			s.close();
+		}
+	}
+
+	public void testMergeEntityWithNonNullablePropSetToNull()
+	{
+		Route route = getUpdatedDetachedEntity();
+		Node node = ( Node ) route.getNodes().iterator().next();
+		node.setName( null );
+
+		Session s = openSession();
+		s.beginTransaction();
+
+		try {
+			s.merge( route );
+			fail( "should have thrown an exception" );
+		}
+		catch ( Exception ex ) {
+			if ( ( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability() ) {
+				assertTrue( ex instanceof PropertyValueException );
+			}
+			else {
+				assertTrue( ex instanceof JDBCException );
+			}
+		}
+		finally {
+			s.getTransaction().rollback();
+			s.close();
+		}
+	}
+
 	public void testMergeRoute()
 	{
 

Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/Node.java
===================================================================
--- core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/Node.java	2010-08-17 15:12:41 UTC (rev 20155)
+++ core/trunk/testsuite/src/test/java/org/hibernate/test/cascade/circle/Node.java	2010-08-17 18:22:38 UTC (rev 20156)
@@ -122,7 +122,10 @@
 		
 		buffer.append( name + " id: " + nodeID );
 		if ( route != null ) {
-			buffer.append( " route name: " ).append( route.getName() ).append( " tour name: " ).append( tour.getName() );
+			buffer.append( " route name: " )
+					.append( route.getName() )
+					.append( " tour name: " )
+					.append( ( tour == null ? "null" : tour.getName() ) );
 		}
 		if ( pickupTransports != null ) {
 			for (Iterator it = pickupTransports.iterator(); it.hasNext();) {



More information about the hibernate-commits mailing list