HHH-5791 : NPE merging transient entity with non-nullable property set to null with delayed insert if check_nullability set to false

This commit is contained in:
Gail Badner 2010-12-17 16:10:33 -08:00
parent 4fa467a8a4
commit 262f25311d
6 changed files with 303 additions and 39 deletions

View File

@ -346,9 +346,19 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
Object propertyFromEntity = persister.getPropertyValue( entity, propertyName, source.getEntityMode() );
Type propertyType = persister.getPropertyType( propertyName );
EntityEntry copyEntry = source.getPersistenceContext().getEntry( copy );
if ( propertyFromCopy == null || ! propertyType.isEntityType() ) {
log.trace( "property '" + copyEntry.getEntityName() + "." + propertyName +
"' is null or not an entity; " + propertyName + " =["+propertyFromCopy+"]");
if ( propertyFromCopy == null ||
propertyFromEntity == null ||
! propertyType.isEntityType() ||
! copyCache.containsKey( propertyFromEntity ) ) {
if ( log.isTraceEnabled() ) {
String fullPropertyName = "property '" + copyEntry.getEntityName() + "." + propertyName;
log.trace( fullPropertyName + " in copy is " + ( propertyFromCopy == null ? "null" : propertyFromCopy ) );
log.trace( fullPropertyName + " in original is " + ( propertyFromCopy == null ? "null" : propertyFromCopy ) );
log.trace( fullPropertyName + ( propertyType.isEntityType() ? " is" : " is not" ) + " an entity type" );
if ( propertyFromEntity != null && ! copyCache.containsKey( propertyFromEntity ) ) {
log.trace( fullPropertyName + " is not in copy cache" );
}
}
if ( isNullabilityCheckedGlobal( source ) ) {
throw ex;
}
@ -358,28 +368,18 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener
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;
if ( log.isTraceEnabled() && propertyFromEntity != null ) {
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; " +
propertyName + " =["+propertyFromEntity+"]");
}
else {
// retry save w/o checking non-nullable properties
// (the failure will be detected later)
saveTransientEntity( copy, entityName, requestedId, source, copyCache, false );
log.trace( "property '" + copyEntry.getEntityName() + "." + propertyName +
"' from original entity is in copyCache and is not in the process of being merged; " +
propertyName + " =["+propertyFromEntity+"]");
}
}
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; " +
propertyName + " =["+propertyFromEntity+"]");
}
else {
log.trace( "property '" + copyEntry.getEntityName() + "." + propertyName +
"' from original entity is in copyCache and is not in the process of being merged; " +
propertyName + " =["+propertyFromEntity+"]");
}
// continue...; we'll find out if it ends up not getting saved later
}

View File

@ -0,0 +1,52 @@
/*
* 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.TransientObjectException;
import org.hibernate.cfg.Configuration;
import org.hibernate.cfg.Environment;
import org.hibernate.testing.junit.functional.FunctionalTestClassTestSuite;
/**
* @author Gail Badner
*/
public class MultiPathCircleCascadeCheckNullFalseDelayedInsertTest extends MultiPathCircleCascadeDelayedInsertTest {
public MultiPathCircleCascadeCheckNullFalseDelayedInsertTest(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( MultiPathCircleCascadeCheckNullFalseDelayedInsertTest.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 MultiPathCircleCascadeCheckNullTrueDelayedInsertTest extends MultiPathCircleCascadeDelayedInsertTest {
public MultiPathCircleCascadeCheckNullTrueDelayedInsertTest(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( MultiPathCircleCascadeCheckNullTrueDelayedInsertTest.class );
}
}

View File

@ -0,0 +1,82 @@
<?xml version="1.0"?>
<!DOCTYPE hibernate-mapping SYSTEM "http://www.hibernate.org/dtd/hibernate-mapping-3.0.dtd" >
<hibernate-mapping package="org.hibernate.test.cascade.circle">
<class name="Route" table="HB_Route">
<id name="routeID" type="long"><generator class="increment"/></id>
<property name="name" type="string" not-null="true"/>
<set name="nodes" inverse="true" cascade="persist,merge,refresh">
<key column="routeID"/>
<one-to-many class="Node"/>
</set>
</class>
<class name="Tour" table="HB_Tour">
<id name="tourID" type="long"><generator class="increment"/></id>
<property name="name" type="string" not-null="true"/>
<set name="nodes" inverse="true" lazy="true" cascade="merge,refresh">
<key column="tourID"/>
<one-to-many class="Node"/>
</set>
</class>
<class name="Transport" table="HB_Transport">
<id name="transportID" type="long"><generator class="increment"/></id>
<property name="name" type="string" not-null="true"/>
<many-to-one name="pickupNode"
column="pickupNodeID"
unique="true"
not-null="true"
cascade="merge,refresh"
lazy="false"/>
<many-to-one name="deliveryNode"
column="deliveryNodeID"
unique="true"
not-null="true"
cascade="merge,refresh"
lazy="false"/>
</class>
<class name="Node" table="HB_Node">
<id name="nodeID" type="long"><generator class="increment"/></id>
<property name="name" type="string" not-null="true"/>
<set name="deliveryTransports" inverse="true" lazy="true" cascade="merge,refresh">
<key column="deliveryNodeID"/>
<one-to-many class="Transport"/>
</set>
<set name="pickupTransports" inverse="true" lazy="true" cascade="merge,refresh">
<key column="pickupNodeID"/>
<one-to-many class="Transport"/>
</set>
<many-to-one name="route"
column="routeID"
unique="false"
not-null="true"
cascade="none"
lazy="false"/>
<many-to-one name="tour"
column="tourID"
unique="false"
not-null="false"
cascade="merge,refresh"
lazy="false"/>
</class>
</hibernate-mapping>

View File

@ -0,0 +1,65 @@
/*
* 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.JDBCException;
import org.hibernate.PropertyValueException;
import org.hibernate.TransientObjectException;
import org.hibernate.testing.junit.functional.FunctionalTestClassTestSuite;
/**
* @author Gail Badner
*/
public class MultiPathCircleCascadeDelayedInsertTest extends MultiPathCircleCascadeTest {
public MultiPathCircleCascadeDelayedInsertTest(String string) {
super(string);
}
public String[] getMappings() {
return new String[] {
"cascade/circle/MultiPathCircleCascadeDelayedInsert.hbm.xml"
};
}
public static Test suite() {
return new FunctionalTestClassTestSuite( MultiPathCircleCascadeDelayedInsertTest.class );
}
protected void checkExceptionFromNullValueForNonNullable(Exception ex, boolean checkNullability, boolean isNullValue ) {
if ( checkNullability ) {
if ( isNullValue ) {
assertTrue( ex instanceof PropertyValueException );
}
else {
assertTrue( ex instanceof TransientObjectException );
}
}
else {
assertTrue( ex instanceof JDBCException || ex instanceof TransientObjectException );
}
}
}

View File

@ -109,15 +109,15 @@ public class MultiPathCircleCascadeTest extends FunctionalTestCase {
try {
s.merge( node );
s.getTransaction().commit();
fail( "should have thrown an exception" );
}
catch ( Exception ex ) {
if ( ( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability() ) {
assertTrue( ex instanceof TransientObjectException );
}
else {
assertTrue( ex instanceof JDBCException );
}
checkExceptionFromNullValueForNonNullable(
ex,
( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability(),
false
);
}
finally {
s.getTransaction().rollback();
@ -138,15 +138,15 @@ public class MultiPathCircleCascadeTest extends FunctionalTestCase {
try {
s.merge( node );
s.getTransaction().commit();
fail( "should have thrown an exception" );
}
catch ( Exception ex ) {
if ( ( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability() ) {
assertTrue( ex instanceof PropertyValueException );
}
else {
assertTrue( ex instanceof JDBCException );
}
checkExceptionFromNullValueForNonNullable(
ex,
( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability(),
true
);
}
finally {
s.getTransaction().rollback();
@ -165,15 +165,15 @@ public class MultiPathCircleCascadeTest extends FunctionalTestCase {
try {
s.merge( route );
s.getTransaction().commit();
fail( "should have thrown an exception" );
}
catch ( Exception ex ) {
if ( ( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability() ) {
assertTrue( ex instanceof PropertyValueException );
}
else {
assertTrue( ex instanceof JDBCException );
}
checkExceptionFromNullValueForNonNullable(
ex,
( ( SessionImplementor ) s ).getFactory().getSettings().isCheckNullability(),
true
);
}
finally {
s.getTransaction().rollback();
@ -531,6 +531,20 @@ public class MultiPathCircleCascadeTest extends FunctionalTestCase {
assertUpdateCount( 1 );
}
protected void checkExceptionFromNullValueForNonNullable(Exception ex, boolean checkNullability, boolean isNullValue ) {
if ( checkNullability ) {
if ( isNullValue ) {
assertTrue( ex instanceof PropertyValueException );
}
else {
assertTrue( ex instanceof TransientObjectException );
}
}
else {
assertTrue( ex instanceof JDBCException );
}
}
protected void clearCounts() {
getSessions().getStatistics().clear();
}