8:22 p.m.
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();) {