HHH-5473 : Default for CHECK_NULLABILITY does not allow merge retries

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@20156 1b8cb986-b30d-0410-93ca-fae66ebed9b2
This commit is contained in:
Gail Badner 2010-08-17 18:22:38 +00:00
parent 35912c5e36
commit 37af87bba0
7 changed files with 303 additions and 33 deletions

View File

@ -89,22 +89,29 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
// 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 + "]" );
}
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();
copyCache = null;
@ -125,11 +132,19 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
// 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 + "]" );
// 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 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
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 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
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,10 +300,20 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
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");
@ -306,15 +337,8 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
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,13 +349,27 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
if ( propertyFromCopy == null || ! propertyType.isEntityType() ) {
log.trace( "property '" + copyEntry.getEntityName() + "." + propertyName +
"' is null or not an entity; " + propertyName + " =["+propertyFromCopy+"]");
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+"]");
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 +
"' from original entity is in copyCache and is in the process of being merged; " +
@ -354,6 +392,36 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
}
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");

View File

@ -198,6 +198,7 @@ public class MultiPathCascadeTest extends FunctionalTestCase {
try {
s.merge( a );
s.merge( h );
s.getTransaction().commit();
fail( "should have thrown TransientObjectException" );
}
catch ( TransientObjectException ex ) {
@ -246,6 +247,7 @@ public class MultiPathCascadeTest extends FunctionalTestCase {
try {
s.merge( a );
s.merge( g );
s.getTransaction().commit();
fail( "should have thrown TransientObjectException" );
}
catch ( TransientObjectException ex ) {
@ -294,6 +296,7 @@ public class MultiPathCascadeTest extends FunctionalTestCase {
try {
s.merge( a );
s.merge( h );
s.getTransaction().commit();
fail( "should have thrown TransientObjectException" );
}
catch ( TransientObjectException ex ) {

View File

@ -66,7 +66,9 @@ public class CascadeMergeToChildBeforeParentTest extends FunctionalTestCase {
return new String[] {
"cascade/circle/CascadeMergeToChildBeforeParent.hbm.xml"
};
}@Override
}
@Override
public void configure(Configuration cfg) {
super.configure( cfg );
cfg.setProperty( Environment.CHECK_NULLABILITY, "true" );

View File

@ -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 );
}
}

View File

@ -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 );
}
}

View File

@ -30,9 +30,13 @@ import java.util.Iterator;
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 class MultiPathCircleCascadeTest extends FunctionalTestCase {
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 @@ public class MultiPathCircleCascadeTest extends FunctionalTestCase {
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()
{

View File

@ -122,7 +122,10 @@ public class Node {
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();) {