HHH-6484 remove orphaned one-to-one when association replaced while managed

This commit is contained in:
Brett Meyer 2013-10-10 14:45:45 -04:00 committed by Brett Meyer
parent 2cc6b08ff2
commit 2fc60011e5
12 changed files with 340 additions and 81 deletions

View File

@ -43,7 +43,7 @@ import org.hibernate.persister.entity.EntityPersister;
/**
* The action for performing an entity deletion.
*/
public final class EntityDeleteAction extends EntityAction {
public class EntityDeleteAction extends EntityAction {
private final Object version;
private final boolean isCascadeDeleteEnabled;
private final Object[] state;

View File

@ -0,0 +1,37 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* Copyright (c) 2013, Red Hat Inc. 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 Inc.
*
* 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.action.internal;
import java.io.Serializable;
import org.hibernate.engine.spi.SessionImplementor;
import org.hibernate.persister.entity.EntityPersister;
public final class OrphanRemovalAction extends EntityDeleteAction {
public OrphanRemovalAction(Serializable id, Object[] state, Object version, Object instance,
EntityPersister persister, boolean isCascadeDeleteEnabled, SessionImplementor session) {
super( id, state, version, instance, persister, isCascadeDeleteEnabled, session );
}
}

View File

@ -29,8 +29,6 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.Stack;
import org.jboss.logging.Logger;
import org.hibernate.HibernateException;
import org.hibernate.collection.spi.PersistentCollection;
import org.hibernate.engine.spi.CascadeStyle;
@ -47,7 +45,11 @@ import org.hibernate.type.AssociationType;
import org.hibernate.type.CollectionType;
import org.hibernate.type.CompositeType;
import org.hibernate.type.EntityType;
import org.hibernate.type.ForeignKeyDirection;
import org.hibernate.type.ManyToOneType;
import org.hibernate.type.OneToOneType;
import org.hibernate.type.Type;
import org.jboss.logging.Logger;
/**
* Delegate responsible for, in conjunction with the various
@ -157,7 +159,7 @@ public final class Cascade {
final Object anything,
final boolean isCascadeDeleteEnabled) throws HibernateException {
if (child!=null) {
if ( child != null ) {
if ( type.isAssociationType() ) {
final AssociationType associationType = (AssociationType) type;
if ( cascadeAssociationNow( associationType ) ) {
@ -175,53 +177,65 @@ public final class Cascade {
cascadeComponent( parent, child, (CompositeType) type, propertyName, anything );
}
}
else {
// potentially we need to handle orphan deletes for one-to-ones here...
if ( isLogicalOneToOne( type ) ) {
// We have a physical or logical one-to-one and from previous checks we know we
// have a null value. See if the attribute cascade settings and action-type require
// orphan checking
if ( style.hasOrphanDelete() && action.deleteOrphans() ) {
// value is orphaned if loaded state for this property shows not null
// because it is currently null.
final EntityEntry entry = eventSource.getPersistenceContext().getEntry( parent );
if ( entry != null && entry.getStatus() != Status.SAVING ) {
final Object loadedValue;
if ( componentPathStack.isEmpty() ) {
// association defined on entity
loadedValue = entry.getLoadedValue( propertyName );
}
else {
// association defined on component
// todo : this is currently unsupported because of the fact that
// we do not know the loaded state of this value properly
// and doing so would be very difficult given how components and
// entities are loaded (and how 'loaded state' is put into the
// EntityEntry). Solutions here are to either:
// 1) properly account for components as a 2-phase load construct
// 2) just assume the association was just now orphaned and
// issue the orphan delete. This would require a special
// set of SQL statements though since we do not know the
// orphaned value, something a delete with a subquery to
// match the owner.
// potentially we need to handle orphan deletes for one-to-ones here...
if ( isLogicalOneToOne( type ) ) {
// We have a physical or logical one-to-one. See if the attribute cascade settings and action-type require
// orphan checking
if ( style.hasOrphanDelete() && action.deleteOrphans() ) {
// value is orphaned if loaded state for this property shows not null
// because it is currently null.
final EntityEntry entry = eventSource.getPersistenceContext().getEntry( parent );
if ( entry != null && entry.getStatus() != Status.SAVING ) {
final Object loadedValue;
if ( componentPathStack.isEmpty() ) {
// association defined on entity
loadedValue = entry.getLoadedValue( propertyName );
}
else {
// association defined on component
// todo : this is currently unsupported because of the fact that
// we do not know the loaded state of this value properly
// and doing so would be very difficult given how components and
// entities are loaded (and how 'loaded state' is put into the
// EntityEntry). Solutions here are to either:
// 1) properly account for components as a 2-phase load construct
// 2) just assume the association was just now orphaned and
// issue the orphan delete. This would require a special
// set of SQL statements though since we do not know the
// orphaned value, something a delete with a subquery to
// match the owner.
// final EntityType entityType = (EntityType) type;
// final String getPropertyPath = composePropertyPath( entityType.getPropertyName() );
loadedValue = null;
}
if ( loadedValue != null ) {
final EntityEntry valueEntry = eventSource
.getPersistenceContext().getEntry(
loadedValue );
// Need to check this in case the context has
// already been flushed. See HHH-7829.
if ( valueEntry != null ) {
final String entityName = valueEntry.getPersister().getEntityName();
if ( LOG.isTraceEnabled() ) {
final Serializable id = valueEntry.getPersister().getIdentifier( loadedValue, eventSource );
final String description = MessageHelper.infoString( entityName, id );
LOG.tracev( "Deleting orphaned entity instance: {0}", description );
}
eventSource.delete( entityName, loadedValue, false, new HashSet() );
loadedValue = null;
}
// orphaned if the association was nulled (child == null) or receives a new value while the
// entity is managed (without first nulling and manually flushing).
if ( child == null || ( loadedValue != null && child != loadedValue ) ) {
final EntityEntry valueEntry = eventSource
.getPersistenceContext().getEntry(
loadedValue );
// Need to check this in case the context has
// already been flushed. See HHH-7829.
if ( valueEntry != null ) {
final String entityName = valueEntry.getPersister().getEntityName();
if ( LOG.isTraceEnabled() ) {
final Serializable id = valueEntry.getPersister().getIdentifier( loadedValue, eventSource );
final String description = MessageHelper.infoString( entityName, id );
LOG.tracev( "Deleting orphaned entity instance: {0}", description );
}
if (type.isAssociationType() && ((AssociationType)type).getForeignKeyDirection().equals(
ForeignKeyDirection.FOREIGN_KEY_TO_PARENT )) {
// If FK direction is to-parent, we must remove the orphan *before* the queued update(s)
// occur. Otherwise, replacing the association on a managed entity, without manually
// nulling and flushing, causes FK constraint violations.
eventSource.removeOrphanBeforeUpdates( entityName, loadedValue );
}
else {
// Else, we must delete after the updates.
eventSource.delete( entityName, loadedValue, isCascadeDeleteEnabled, new HashSet() );
}
}
}

View File

@ -50,6 +50,7 @@ import org.hibernate.action.internal.EntityDeleteAction;
import org.hibernate.action.internal.EntityIdentityInsertAction;
import org.hibernate.action.internal.EntityInsertAction;
import org.hibernate.action.internal.EntityUpdateAction;
import org.hibernate.action.internal.OrphanRemovalAction;
import org.hibernate.action.internal.QueuedOperationCollectionAction;
import org.hibernate.action.internal.UnresolvedEntityInsertActions;
import org.hibernate.action.spi.AfterTransactionCompletionProcess;
@ -94,6 +95,10 @@ public class ActionQueue {
private final ExecutableList<QueuedOperationCollectionAction> collectionQueuedOps;
private final ExecutableList<CollectionRemoveAction> collectionRemovals;
// TODO: The removeOrphan concept is a temporary "hack" for HHH-6484. This should be removed once action/task
// ordering is improved.
private final ExecutableList<OrphanRemovalAction> orphanRemovals;
// an immutable array holding all 7 ExecutionLists in execution order
private final List<ExecutableList<?>> executableLists;
@ -119,8 +124,11 @@ public class ActionQueue {
collectionUpdates = new ExecutableList<CollectionUpdateAction>();
collectionQueuedOps = new ExecutableList<QueuedOperationCollectionAction>();
orphanRemovals = new ExecutableList<OrphanRemovalAction>();
// Important: these lists are in execution order
List<ExecutableList<?>> tmp = new ArrayList<ExecutableList<?>>( 7 );
tmp.add( orphanRemovals );
tmp.add( insertions );
tmp.add( updates );
// do before actions are handled in the other collection queues
@ -211,6 +219,15 @@ public class ActionQueue {
deletions.add( action );
}
/**
* Adds an orphan removal action
*
* @param action The action representing the orphan removal
*/
public void addAction(OrphanRemovalAction action) {
orphanRemovals.add( action );
}
/**
* Adds an entity update action
*
@ -369,7 +386,7 @@ public class ActionQueue {
* @return {@code true} if insertions or deletions are currently queued; {@code false} otherwise.
*/
public boolean areInsertionsOrDeletionsQueued() {
return !insertions.isEmpty() || !unresolvedInsertions.isEmpty() || !deletions.isEmpty();
return !insertions.isEmpty() || !unresolvedInsertions.isEmpty() || !deletions.isEmpty() || !orphanRemovals.isEmpty();
}
/**
@ -492,6 +509,7 @@ public class ActionQueue {
return "ActionQueue[insertions=" + insertions
+ " updates=" + updates
+ " deletions=" + deletions
+ " orphanRemovals=" + orphanRemovals
+ " collectionCreations=" + collectionCreations
+ " collectionRemovals=" + collectionRemovals
+ " collectionUpdates=" + collectionUpdates
@ -513,7 +531,7 @@ public class ActionQueue {
}
public int numberOfDeletions() {
return deletions.size();
return deletions.size() + orphanRemovals.size();
}
public int numberOfUpdates() {
@ -577,6 +595,13 @@ public class ActionQueue {
return;
}
}
for ( int i = 0; i < orphanRemovals.size(); i++ ) {
EntityDeleteAction action = orphanRemovals.get( i );
if ( action.getInstance() == rescuedEntity ) {
orphanRemovals.remove( i );
return;
}
}
throw new AssertionFailure( "Unable to perform un-delete for instance " + entry.getEntityName() );
}

View File

@ -26,13 +26,12 @@ package org.hibernate.event.internal;
import java.io.Serializable;
import java.util.Set;
import org.jboss.logging.Logger;
import org.hibernate.CacheMode;
import org.hibernate.HibernateException;
import org.hibernate.LockMode;
import org.hibernate.TransientObjectException;
import org.hibernate.action.internal.EntityDeleteAction;
import org.hibernate.action.internal.OrphanRemovalAction;
import org.hibernate.classic.Lifecycle;
import org.hibernate.engine.internal.Cascade;
import org.hibernate.engine.internal.CascadePoint;
@ -52,6 +51,7 @@ import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.pretty.MessageHelper;
import org.hibernate.type.Type;
import org.hibernate.type.TypeHelper;
import org.jboss.logging.Logger;
/**
* Defines the default delete event listener used by hibernate for deleting entities
@ -159,7 +159,8 @@ public class DefaultDeleteEventListener implements DeleteEventListener {
return;
}
deleteEntity( source, entity, entityEntry, event.isCascadeDeleteEnabled(), persister, transientEntities );
deleteEntity( source, entity, entityEntry, event.isCascadeDeleteEnabled(),
event.isOrphanRemovalBeforeUpdates(), persister, transientEntities );
if ( source.getFactory().getSettings().isIdentifierRollbackEnabled() ) {
persister.resetIdentifier( entity, id, version, source );
@ -227,6 +228,7 @@ public class DefaultDeleteEventListener implements DeleteEventListener {
final Object entity,
final EntityEntry entityEntry,
final boolean isCascadeDeleteEnabled,
final boolean isOrphanRemovalBeforeUpdates,
final EntityPersister persister,
final Set transientEntities) {
@ -268,18 +270,35 @@ public class DefaultDeleteEventListener implements DeleteEventListener {
new Nullability( session ).checkNullability( entityEntry.getDeletedState(), persister, true );
persistenceContext.getNullifiableEntityKeys().add( key );
// Ensures that containing deletions happen before sub-deletions
session.getActionQueue().addAction(
new EntityDeleteAction(
entityEntry.getId(),
deletedState,
version,
entity,
persister,
isCascadeDeleteEnabled,
session
)
);
if (isOrphanRemovalBeforeUpdates) {
// TODO: The removeOrphan concept is a temporary "hack" for HHH-6484. This should be removed once action/task
// ordering is improved.
session.getActionQueue().addAction(
new OrphanRemovalAction(
entityEntry.getId(),
deletedState,
version,
entity,
persister,
isCascadeDeleteEnabled,
session
)
);
}
else {
// Ensures that containing deletions happen before sub-deletions
session.getActionQueue().addAction(
new EntityDeleteAction(
entityEntry.getId(),
deletedState,
version,
entity,
persister,
isCascadeDeleteEnabled,
session
)
);
}
cascadeAfterDelete( session, persister, entity, transientEntities );

View File

@ -32,6 +32,9 @@ public class DeleteEvent extends AbstractEvent {
private Object object;
private String entityName;
private boolean cascadeDeleteEnabled;
// TODO: The removeOrphan concept is a temporary "hack" for HHH-6484. This should be removed once action/task
// ordering is improved.
private boolean orphanRemovalBeforeUpdates;
/**
* Constructs a new DeleteEvent instance.
@ -54,10 +57,18 @@ public class DeleteEvent extends AbstractEvent {
this.entityName = entityName;
}
public DeleteEvent(String entityName, Object object, boolean isCascadeDeleteEnabled, EventSource source) {
public DeleteEvent(String entityName, Object object, boolean cascadeDeleteEnabled, EventSource source) {
this(object, source);
this.entityName = entityName;
cascadeDeleteEnabled = isCascadeDeleteEnabled;
this.cascadeDeleteEnabled = cascadeDeleteEnabled;
}
public DeleteEvent(String entityName, Object object, boolean cascadeDeleteEnabled,
boolean orphanRemovalBeforeUpdates, EventSource source) {
this(object, source);
this.entityName = entityName;
this.cascadeDeleteEnabled = cascadeDeleteEnabled;
this.orphanRemovalBeforeUpdates = orphanRemovalBeforeUpdates;
}
/**
@ -77,4 +88,8 @@ public class DeleteEvent extends AbstractEvent {
return cascadeDeleteEnabled;
}
public boolean isOrphanRemovalBeforeUpdates() {
return orphanRemovalBeforeUpdates;
}
}

View File

@ -76,5 +76,11 @@ public interface EventSource extends SessionImplementor, Session {
* Cascade delete an entity instance
*/
public void delete(String entityName, Object child, boolean isCascadeDeleteEnabled, Set transientEntities);
/**
* A specialized type of deletion for orphan removal that must occur prior to queued inserts and updates.
*/
// TODO: The removeOrphan concept is a temporary "hack" for HHH-6484. This should be removed once action/task
// ordering is improved.
public void removeOrphanBeforeUpdates(String entityName, Object child);
}

View File

@ -958,6 +958,12 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
fireDelete( new DeleteEvent( entityName, object, isCascadeDeleteEnabled, this ), transientEntities );
}
// TODO: The removeOrphan concept is a temporary "hack" for HHH-6484. This should be removed once action/task
// ordering is improved.
public void removeOrphanBeforeUpdates(String entityName, Object child) {
fireDelete( new DeleteEvent( entityName, child, false, true, this ) );
}
private void fireDelete(DeleteEvent event) {
errorIfClosed();
checkTransactionSynchStatus();

View File

@ -28,6 +28,7 @@ import java.util.List;
import org.junit.Test;
import org.hibernate.Session;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
import static org.junit.Assert.assertEquals;
@ -91,4 +92,38 @@ public class DeleteOneToOneOrphansTest extends BaseCoreFunctionalTestCase {
cleanupData();
}
@Test
@TestForIssue(jiraKey = "HHH-6484")
public void testReplacedWhileManaged() {
createData();
Session session = openSession();
session.beginTransaction();
List results = session.createQuery( "from EmployeeInfo" ).list();
assertEquals( 1, results.size() );
results = session.createQuery( "from Employee" ).list();
assertEquals( 1, results.size() );
Employee emp = (Employee) results.get( 0 );
assertNotNull( emp.getInfo() );
// Replace with a new EmployeeInfo instance
emp.setInfo( new EmployeeInfo( emp ) );
session.getTransaction().commit();
session.close();
session = openSession();
session.beginTransaction();
emp = (Employee) session.get( Employee.class, emp.getId() );
assertNotNull( emp.getInfo() );
results = session.createQuery( "from EmployeeInfo" ).list();
assertEquals( 1, results.size() );
results = session.createQuery( "from Employee" ).list();
assertEquals( 1, results.size() );
session.getTransaction().commit();
session.close();
cleanupData();
}
}

View File

@ -28,6 +28,7 @@ import java.util.List;
import org.junit.Test;
import org.hibernate.Session;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
import static org.junit.Assert.assertEquals;
@ -56,8 +57,8 @@ public class DeleteOneToOneOrphansTest extends BaseCoreFunctionalTestCase {
private void cleanupData() {
Session session = openSession();
session.beginTransaction();
session.createQuery( "delete EmployeeInfo" ).executeUpdate();
session.createQuery( "delete Employee" ).executeUpdate();
session.createQuery( "delete EmployeeInfo" ).executeUpdate();
session.getTransaction().commit();
session.close();
}
@ -91,4 +92,38 @@ public class DeleteOneToOneOrphansTest extends BaseCoreFunctionalTestCase {
cleanupData();
}
@Test
@TestForIssue(jiraKey = "HHH-6484")
public void testReplacedWhileManaged() {
createData();
Session session = openSession();
session.beginTransaction();
List results = session.createQuery( "from EmployeeInfo" ).list();
assertEquals( 1, results.size() );
results = session.createQuery( "from Employee" ).list();
assertEquals( 1, results.size() );
Employee emp = (Employee) results.get( 0 );
assertNotNull( emp.getInfo() );
// Replace with a new EmployeeInfo instance
emp.setInfo( new EmployeeInfo( 2L, 2L ) );
session.getTransaction().commit();
session.close();
session = openSession();
session.beginTransaction();
emp = (Employee) session.get( Employee.class, emp.getId() );
assertNotNull( emp.getInfo() );
results = session.createQuery( "from EmployeeInfo" ).list();
assertEquals( 1, results.size() );
results = session.createQuery( "from Employee" ).list();
assertEquals( 1, results.size() );
session.getTransaction().commit();
session.close();
cleanupData();
}
}

View File

@ -28,6 +28,7 @@ import java.util.List;
import org.junit.Test;
import org.hibernate.Session;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
import static org.junit.Assert.assertEquals;
@ -55,8 +56,8 @@ public class DeleteOneToOneOrphansTest extends BaseCoreFunctionalTestCase {
private void cleanupData() {
Session session = openSession();
session.beginTransaction();
session.createQuery( "delete EmployeeInfo" ).executeUpdate();
session.createQuery( "delete Employee" ).executeUpdate();
session.createQuery( "delete EmployeeInfo" ).executeUpdate();
session.getTransaction().commit();
session.close();
}
@ -90,4 +91,38 @@ public class DeleteOneToOneOrphansTest extends BaseCoreFunctionalTestCase {
cleanupData();
}
@Test
@TestForIssue(jiraKey = "HHH-6484")
public void testReplacedWhileManaged() {
createData();
Session session = openSession();
session.beginTransaction();
List results = session.createQuery( "from EmployeeInfo" ).list();
assertEquals( 1, results.size() );
results = session.createQuery( "from Employee" ).list();
assertEquals( 1, results.size() );
Employee emp = (Employee) results.get( 0 );
assertNotNull( emp.getInfo() );
// Replace with a new EmployeeInfo instance
emp.setInfo( new EmployeeInfo( emp ) );
session.getTransaction().commit();
session.close();
session = openSession();
session.beginTransaction();
emp = (Employee) session.get( Employee.class, emp.getId() );
assertNotNull( emp.getInfo() );
results = session.createQuery( "from EmployeeInfo" ).list();
assertEquals( 1, results.size() );
results = session.createQuery( "from Employee" ).list();
assertEquals( 1, results.size() );
session.getTransaction().commit();
session.close();
cleanupData();
}
}

View File

@ -23,19 +23,17 @@
*/
package org.hibernate.test.orphan.one2one.fk.reversed.unidirectional;
import java.util.List;
import org.junit.Test;
import org.hibernate.Session;
import org.hibernate.testing.FailureExpected;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import java.util.List;
import org.hibernate.Session;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
import org.junit.Test;
/**
* @author Steve Ebersole
*/
@ -57,8 +55,8 @@ public class DeleteOneToOneOrphansTest extends BaseCoreFunctionalTestCase {
private void cleanupData() {
Session session = openSession();
session.beginTransaction();
session.createQuery( "delete EmployeeInfo" ).executeUpdate();
session.createQuery( "delete Employee" ).executeUpdate();
session.createQuery( "delete EmployeeInfo" ).executeUpdate();
session.getTransaction().commit();
session.close();
}
@ -137,4 +135,38 @@ public class DeleteOneToOneOrphansTest extends BaseCoreFunctionalTestCase {
cleanupData();
}
@Test
@TestForIssue(jiraKey = "HHH-6484")
public void testReplacedWhileManaged() {
createData();
Session session = openSession();
session.beginTransaction();
List results = session.createQuery( "from EmployeeInfo" ).list();
assertEquals( 1, results.size() );
results = session.createQuery( "from Employee" ).list();
assertEquals( 1, results.size() );
Employee emp = (Employee) results.get( 0 );
assertNotNull( emp.getInfo() );
// Replace with a new EmployeeInfo instance
emp.setInfo( new EmployeeInfo() );
session.getTransaction().commit();
session.close();
session = openSession();
session.beginTransaction();
emp = (Employee) session.get( Employee.class, emp.getId() );
assertNotNull( emp.getInfo() );
results = session.createQuery( "from EmployeeInfo" ).list();
assertEquals( 1, results.size() );
results = session.createQuery( "from Employee" ).list();
assertEquals( 1, results.size() );
session.getTransaction().commit();
session.close();
cleanupData();
}
}