HHH-5472 : Delay saving an entity if it does not cascade the save to non-nullable transient entities

This commit is contained in:
Gail Badner 2012-01-11 04:18:35 -08:00
parent ea7afb2683
commit 66a9f21e89
11 changed files with 172 additions and 131 deletions

View File

@ -75,11 +75,11 @@ public abstract class AbstractEntityInsertAction extends EntityAction {
/**
* Returns the entity state.
*
* NOTE: calling {@link #nullifyTransientReferences()} can modify the
* NOTE: calling {@link #nullifyTransientReferencesIfNotAlready} can modify the
* entity state.
* @return the entity state.
*
* @see {@link #nullifyTransientReferences()}
* @see {@link #nullifyTransientReferencesIfNotAlready}
*/
public Object[] getState() {
return state;
@ -108,39 +108,32 @@ public abstract class AbstractEntityInsertAction extends EntityAction {
);
}
/**
* Have transient references been nullified?
*
* @return true, if transient references have been nullified; false, otherwise.
*
* @see {@link #nullifyTransientReferences()}
*/
protected final boolean areTransientReferencesNullified() {
return areTransientReferencesNullified;
}
/**
* Nullifies any references to transient entities in the entity state
* maintained by this action. This method must be called when an entity
* is made "managed" or when this action is executed, whichever is first.
* maintained by this action. References to transient entities
* should be nullified when an entity is made "managed" or when this
* action is executed, whichever is first.
* <p/>
* References will only be nullified the first time this method is
* called for a this object, so it can safely be called both when
* the entity is made "managed" and when this action is executed.
*
* @see {@link #areTransientReferencesNullified()}
* @see {@link #makeEntityManaged() }
*/
protected final void nullifyTransientReferences() {
new ForeignKeys.Nullifier( getInstance(), false, isEarlyInsert(), getSession() )
.nullifyTransientReferences( getState(), getPersister().getPropertyTypes() );
new Nullability( getSession() ).checkNullability( getState(), getPersister(), false );
areTransientReferencesNullified = true;
protected final void nullifyTransientReferencesIfNotAlready() {
if ( ! areTransientReferencesNullified ) {
new ForeignKeys.Nullifier( getInstance(), false, isEarlyInsert(), getSession() )
.nullifyTransientReferences( getState(), getPersister().getPropertyTypes() );
new Nullability( getSession() ).checkNullability( getState(), getPersister(), false );
areTransientReferencesNullified = true;
}
}
/**
* Make the entity "managed" by the persistence context.
*/
public final void makeEntityManaged() {
if ( !areTransientReferencesNullified ) {
nullifyTransientReferences();
}
nullifyTransientReferencesIfNotAlready();
Object version = Versioning.getVersion( getState(), getPersister() );
getSession().getPersistenceContext().addEntity(
getInstance(),

View File

@ -66,9 +66,7 @@ public final class EntityIdentityInsertAction extends AbstractEntityInsertAction
@Override
public void execute() throws HibernateException {
if ( ! areTransientReferencesNullified() ) {
nullifyTransientReferences();
}
nullifyTransientReferencesIfNotAlready();
final EntityPersister persister = getPersister();
final SessionImplementor session = getSession();
@ -87,10 +85,9 @@ public final class EntityIdentityInsertAction extends AbstractEntityInsertAction
//need to do that here rather than in the save event listener to let
//the post insert events to have a id-filled entity when IDENTITY is used (EJB3)
persister.setIdentifier( instance, generatedId, session );
getSession().getPersistenceContext().registerInsertedKey( getPersister(), generatedId );
// TODO: decide where to do this...
entityKey = getSession().generateEntityKey( generatedId, persister );
getSession().getPersistenceContext().checkUniqueness( entityKey, getInstance() );
session.getPersistenceContext().registerInsertedKey( getPersister(), generatedId );
entityKey = session.generateEntityKey( generatedId, persister );
session.getPersistenceContext().checkUniqueness( entityKey, getInstance() );
}

View File

@ -71,9 +71,7 @@ public final class EntityInsertAction extends AbstractEntityInsertAction {
@Override
public void execute() throws HibernateException {
if ( ! areTransientReferencesNullified() ) {
nullifyTransientReferences();
}
nullifyTransientReferencesIfNotAlready();
EntityPersister persister = getPersister();
SessionImplementor session = getSession();

View File

@ -26,6 +26,7 @@ package org.hibernate.action.internal;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Map;
@ -34,7 +35,7 @@ import java.util.TreeSet;
import org.jboss.logging.Logger;
import org.hibernate.TransientObjectException;
import org.hibernate.PropertyValueException;
import org.hibernate.engine.internal.NonNullableTransientDependencies;
import org.hibernate.engine.spi.EntityEntry;
import org.hibernate.engine.spi.SessionImplementor;
@ -61,12 +62,12 @@ public class UnresolvedEntityInsertActions {
CoreMessageLogger.class,
UnresolvedEntityInsertActions.class.getName()
);
private static final int INIT_LIST_SIZE = 5;
private static final int INIT_SIZE = 5;
private final Map<AbstractEntityInsertAction,NonNullableTransientDependencies> dependenciesByAction =
new IdentityHashMap<AbstractEntityInsertAction,NonNullableTransientDependencies>( INIT_LIST_SIZE );
new IdentityHashMap<AbstractEntityInsertAction,NonNullableTransientDependencies>( INIT_SIZE );
private final Map<Object,Set<AbstractEntityInsertAction>> dependentActionsByTransientEntity =
new IdentityHashMap<Object,Set<AbstractEntityInsertAction>>( INIT_LIST_SIZE );
new IdentityHashMap<Object,Set<AbstractEntityInsertAction>>( INIT_SIZE );
/**
* Add an unresolved insert action.
@ -102,6 +103,69 @@ public class UnresolvedEntityInsertActions {
return dependenciesByAction.keySet();
}
/**
* Throws {@link org.hibernate.PropertyValueException} if there are any unresolved
* entity insert actions that depend on non-nullable associations with
* a transient entity. This method should be called on completion of
* an operation (after all cascades are completed) that saves an entity.
*
* @throws org.hibernate.PropertyValueException if there are any unresolved entity
* insert actions; {@link org.hibernate.PropertyValueException#getEntityName()}
* and {@link org.hibernate.PropertyValueException#getPropertyName()} will
* return the entity name and property value for the first unresolved
* entity insert action.
*/
public void checkNoUnresolvedActionsAfterOperation() throws PropertyValueException {
if ( isEmpty() ) {
LOG.trace( "No entity insert actions have non-nullable, transient entity dependencies." );
}
else {
AbstractEntityInsertAction firstDependentAction =
dependenciesByAction.keySet().iterator().next();
logCannotResolveNonNullableTransientDependencies( firstDependentAction.getSession() );
NonNullableTransientDependencies nonNullableTransientDependencies =
dependenciesByAction.get( firstDependentAction );
Object firstTransientDependency =
nonNullableTransientDependencies.getNonNullableTransientEntities().iterator().next();
String firstPropertyPath =
nonNullableTransientDependencies.getNonNullableTransientPropertyPaths( firstTransientDependency ).iterator().next();
throw new PropertyValueException(
"Not-null property references a transient value - transient instance must be saved before current operation",
firstDependentAction.getEntityName(),
firstPropertyPath
);
}
}
private void logCannotResolveNonNullableTransientDependencies(SessionImplementor session) {
for ( Map.Entry<Object,Set<AbstractEntityInsertAction>> entry : dependentActionsByTransientEntity.entrySet() ) {
Object transientEntity = entry.getKey();
String transientEntityName = session.guessEntityName( transientEntity );
Serializable transientEntityId = session.getFactory().getEntityPersister( transientEntityName ).getIdentifier( transientEntity, session );
String transientEntityString = MessageHelper.infoString( transientEntityName, transientEntityId );
Set<String> dependentEntityStrings = new TreeSet<String>();
Set<String> nonNullableTransientPropertyPaths = new TreeSet<String>();
for ( AbstractEntityInsertAction dependentAction : entry.getValue() ) {
dependentEntityStrings.add( MessageHelper.infoString( dependentAction.getEntityName(), dependentAction.getId() ) );
for ( String path : dependenciesByAction.get( dependentAction ).getNonNullableTransientPropertyPaths( transientEntity ) ) {
String fullPath = new StringBuilder( dependentAction.getEntityName().length() + path.length() + 1 )
.append( dependentAction.getEntityName() )
.append( '.' )
.append( path )
.toString();
nonNullableTransientPropertyPaths.add( fullPath );
}
}
LOG.cannotResolveNonNullableTransientDependencies(
transientEntityString,
dependentEntityStrings,
nonNullableTransientPropertyPaths
);
}
}
/**
* Returns true if there are no unresolved entity insert actions.
* @return true, if there are no unresolved entity insert actions; false, otherwise.
@ -151,16 +215,29 @@ public class UnresolvedEntityInsertActions {
return Collections.emptySet(); //NOTE EARLY EXIT!
}
Set<AbstractEntityInsertAction> resolvedActions = new IdentitySet( );
if ( LOG.isTraceEnabled() ) {
LOG.tracev(
"Unresolved inserts before resolving [{0}]: [{1}]",
MessageHelper.infoString( entityEntry.getEntityName(), entityEntry.getId() ),
toString()
);
}
for ( AbstractEntityInsertAction dependentAction : dependentActions ) {
if ( LOG.isTraceEnabled() ) {
LOG.tracev(
"Resolving insert [{0}] dependency on [{1}]",
MessageHelper.infoString( dependentAction.getEntityName(), dependentAction.getId() ),
MessageHelper.infoString( entityEntry.getEntityName(), entityEntry.getId() )
);
}
NonNullableTransientDependencies dependencies = dependenciesByAction.get( dependentAction );
dependencies.resolveNonNullableTransientEntity( managedEntity );
if ( dependencies.isEmpty() ) {
if ( LOG.isTraceEnabled() ) {
LOG.tracev(
"Entity insert [{0}] only depended on [{1}]; removing from [{2}]",
"Resolving insert [{0}] (only depended on [{1}])",
dependentAction,
MessageHelper.infoString( entityEntry.getEntityName(), entityEntry.getId() ),
getClass().getSimpleName()
MessageHelper.infoString( entityEntry.getEntityName(), entityEntry.getId() )
);
}
// dependentAction only depended on managedEntity..
@ -168,8 +245,12 @@ public class UnresolvedEntityInsertActions {
resolvedActions.add( dependentAction );
}
}
if ( LOG.isTraceEnabled() && ! resolvedActions.isEmpty() ) {
LOG.tracev( "Remaining unresolved dependencies: ", toString() );
if ( LOG.isTraceEnabled() ) {
LOG.tracev(
"Unresolved inserts after resolving [{0}]: [{1}]",
MessageHelper.infoString( entityEntry.getEntityName(), entityEntry.getId() ),
toString()
);
}
return resolvedActions;
}
@ -182,64 +263,6 @@ public class UnresolvedEntityInsertActions {
dependentActionsByTransientEntity.clear();
}
/**
* Throw TransientObjectException if there are any unresolved entity
* insert actions.
*
* @param session - the session
*
* @throws TransientObjectException if there are any unresolved
* entity insert actions.
*/
public void throwTransientObjectExceptionIfNotEmpty(SessionImplementor session) {
if ( isEmpty() ) {
return; // EARLY RETURN
}
StringBuilder sb = new StringBuilder(
"Could not save one or more entities because of non-nullable associations with unsaved transient instance(s); save these transient instance(s) before saving the dependent entities.\n"
);
boolean firstTransientEntity = true;
for ( Map.Entry<Object,Set<AbstractEntityInsertAction>> entry : dependentActionsByTransientEntity.entrySet() ) {
if ( firstTransientEntity ) {
firstTransientEntity = false;
}
else {
sb.append( '\n' );
}
Object transientEntity = entry.getKey();
Set<String> propertyPaths = new TreeSet<String>();
for ( AbstractEntityInsertAction dependentAction : entry.getValue() ) {
for ( String fullPropertyPaths :
dependenciesByAction.get( dependentAction ).getNonNullableTransientPropertyPaths( transientEntity ) ) {
propertyPaths.add( fullPropertyPaths );
}
}
sb.append( "Non-nullable association" );
if ( propertyPaths.size() > 1 ) {
sb.append( 's' );
}
sb.append( " (" );
boolean firstPropertyPath = true;
for ( String propertyPath : propertyPaths ) {
if ( firstPropertyPath ) {
firstPropertyPath = false;
}
else {
sb.append( ", " );
}
sb.append( propertyPath );
}
sb.append( ") depend" );
if( propertyPaths.size() == 1 ) {
sb.append( 's' );
}
sb.append( " on unsaved transient entity: " )
.append( session.guessEntityName( transientEntity ) )
.append( '.' );
}
throw new TransientObjectException( sb.toString() );
}
@Override
public String toString() {
StringBuilder sb = new StringBuilder( getClass().getSimpleName() )

View File

@ -288,7 +288,6 @@ public final class ForeignKeys {
NonNullableTransientDependencies nonNullableTransientEntities = new NonNullableTransientDependencies();
for ( int i = 0; i < types.length; i++ ) {
collectNonNullableTransientEntities(
entityName,
nullifier,
i,
values[i],
@ -303,7 +302,6 @@ public final class ForeignKeys {
}
private static void collectNonNullableTransientEntities(
String entityName,
Nullifier nullifier,
int i,
Object value,
@ -320,13 +318,13 @@ public final class ForeignKeys {
if ( ! isNullable &&
! entityType.isOneToOne() &&
nullifier.isNullifiable( entityType.getAssociatedEntityName(), value ) ) {
nonNullableTransientEntities.add( entityName, propertyName, value );
nonNullableTransientEntities.add( propertyName, value );
}
}
else if ( type.isAnyType() ) {
if ( ! isNullable &&
nullifier.isNullifiable( null, value ) ) {
nonNullableTransientEntities.add( entityName, propertyName, value );
nonNullableTransientEntities.add( propertyName, value );
}
}
else if ( type.isComponentType() ) {
@ -338,7 +336,6 @@ public final class ForeignKeys {
Type[] subtypes = actype.getSubtypes();
for ( int j = 0; j < subvalues.length; j++ ) {
collectNonNullableTransientEntities(
entityName,
nullifier,
j,
subvalues[j],

View File

@ -31,24 +31,25 @@ import java.util.Set;
import org.hibernate.engine.spi.SessionImplementor;
/**
* Tracks non-nullable transient entities that would cause a particular entity insert to fail.
*
* @author Gail Badner
*/
public class NonNullableTransientDependencies {
// Multiple property paths can refer to the same transient entity, so use Set<String>
// for the map value.
private final Map<Object,Set<String>> propertyPathsByTransientEntity =
new IdentityHashMap<Object,Set<String>>();
/* package-protected */
void add(String entityName, String propertyName, Object transientEntity) {
void add(String propertyName, Object transientEntity) {
Set<String> propertyPaths = propertyPathsByTransientEntity.get( transientEntity );
if ( propertyPaths == null ) {
propertyPaths = new HashSet<String>();
propertyPathsByTransientEntity.put( transientEntity, propertyPaths );
}
StringBuilder sb = new StringBuilder( entityName.length() + propertyName.length() + 1 )
.append( entityName )
.append( '.' )
.append( propertyName );
propertyPaths.add( sb.toString() );
propertyPaths.add( propertyName );
}
public Iterable<Object> getNonNullableTransientEntities() {

View File

@ -38,6 +38,7 @@ import org.jboss.logging.Logger;
import org.hibernate.AssertionFailure;
import org.hibernate.HibernateException;
import org.hibernate.PropertyValueException;
import org.hibernate.action.internal.AbstractEntityInsertAction;
import org.hibernate.action.internal.BulkOperationCleanupAction;
import org.hibernate.action.internal.CollectionAction;
@ -212,12 +213,31 @@ public class ActionQueue {
}
}
/**
* Are there unresolved entity insert actions that depend on non-nullable
* associations with a transient entity?
* @return true, if there are unresolved entity insert actions that depend on
* non-nullable associations with a transient entity;
* false, otherwise
*/
public boolean hasUnresolvedEntityInsertActions() {
return ! unresolvedInsertions.isEmpty();
}
public void checkNoUnresolvedEntityInsertActions() {
unresolvedInsertions.throwTransientObjectExceptionIfNotEmpty( session );
/**
* Throws {@link org.hibernate.PropertyValueException} if there are any unresolved
* entity insert actions that depend on non-nullable associations with
* a transient entity. This method should be called on completion of
* an operation (after all cascades are completed) that saves an entity.
*
* @throws org.hibernate.PropertyValueException if there are any unresolved entity
* insert actions; {@link org.hibernate.PropertyValueException#getEntityName()}
* and {@link org.hibernate.PropertyValueException#getPropertyName()} will
* return the entity name and property value for the first unresolved
* entity insert action.
*/
public void checkNoUnresolvedActionsAfterOperation() throws PropertyValueException {
unresolvedInsertions.checkNoUnresolvedActionsAfterOperation();
}
public void addAction(BulkOperationCleanupAction cleanupAction) {
@ -247,7 +267,11 @@ public class ActionQueue {
* @throws HibernateException error executing queued actions.
*/
public void executeActions() throws HibernateException {
checkNoUnresolvedEntityInsertActions();
if ( ! unresolvedInsertions.isEmpty() ) {
throw new IllegalStateException(
"About to execute actions, but there are unresolved entity insert actions."
);
}
executeActions( insertions );
executeActions( updates );
executeActions( collectionRemovals );

View File

@ -1556,4 +1556,10 @@ public interface CoreMessageLogger extends BasicLogger {
"or passivated, specify a unique value for property '%s'", id = 436)
void entityManagerFactoryAlreadyRegistered(String emfName, String propertyName);
@LogMessage(level = WARN)
@Message(value = "Attempting to save one or more entities that have a non-nullable association with an unsaved transient entity. The unsaved transient entity must be saved in an operation prior to saving these dependent entities.\n" +
"\tUnsaved transient entity: (%s)\n\tDependent entities: (%s)\n\tNon-nullable association(s): (%s)" , id = 437)
void cannotResolveNonNullableTransientDependencies(String transientEntityString,
Set<String> dependentEntityStrings,
Set<String> nonNullableAssociationPaths);
}

View File

@ -606,15 +606,15 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
}
}
private void checkDelayedActionStatusBeforeOperation() {
private void checkNoUnresolvedActionsBeforeOperation() {
if ( persistenceContext.getCascadeLevel() == 0 && actionQueue.hasUnresolvedEntityInsertActions() ) {
throw new IllegalStateException( "There are delayed insert actions before operation as cascade level 0." );
}
}
private void checkDelayedActionStatusAfterOperation() {
private void checkNoUnresolvedActionsAfterOperation() {
if ( persistenceContext.getCascadeLevel() == 0 ) {
actionQueue.checkNoUnresolvedEntityInsertActions();
actionQueue.checkNoUnresolvedActionsAfterOperation();
}
}
@ -631,11 +631,11 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
private void fireSaveOrUpdate(SaveOrUpdateEvent event) {
errorIfClosed();
checkTransactionSynchStatus();
checkDelayedActionStatusBeforeOperation();
checkNoUnresolvedActionsBeforeOperation();
for ( SaveOrUpdateEventListener listener : listeners( EventType.SAVE_UPDATE ) ) {
listener.onSaveOrUpdate( event );
}
checkDelayedActionStatusAfterOperation();
checkNoUnresolvedActionsAfterOperation();
}
private <T> Iterable<T> listeners(EventType<T> type) {
@ -660,11 +660,11 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
private Serializable fireSave(SaveOrUpdateEvent event) {
errorIfClosed();
checkTransactionSynchStatus();
checkDelayedActionStatusBeforeOperation();
checkNoUnresolvedActionsBeforeOperation();
for ( SaveOrUpdateEventListener listener : listeners( EventType.SAVE ) ) {
listener.onSaveOrUpdate( event );
}
checkDelayedActionStatusAfterOperation();
checkNoUnresolvedActionsAfterOperation();
return event.getResultId();
}
@ -682,11 +682,11 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
private void fireUpdate(SaveOrUpdateEvent event) {
errorIfClosed();
checkTransactionSynchStatus();
checkDelayedActionStatusBeforeOperation();
checkNoUnresolvedActionsBeforeOperation();
for ( SaveOrUpdateEventListener listener : listeners( EventType.UPDATE ) ) {
listener.onSaveOrUpdate( event );
}
checkDelayedActionStatusAfterOperation();
checkNoUnresolvedActionsAfterOperation();
}
@ -747,11 +747,11 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
private void firePersist(PersistEvent event) {
errorIfClosed();
checkTransactionSynchStatus();
checkDelayedActionStatusBeforeOperation();
checkNoUnresolvedActionsBeforeOperation();
for ( PersistEventListener listener : listeners( EventType.PERSIST ) ) {
listener.onPersist( event );
}
checkDelayedActionStatusAfterOperation();
checkNoUnresolvedActionsAfterOperation();
}
@ -782,11 +782,11 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
private void firePersistOnFlush(PersistEvent event) {
errorIfClosed();
checkTransactionSynchStatus();
checkDelayedActionStatusBeforeOperation();
checkNoUnresolvedActionsBeforeOperation();
for ( PersistEventListener listener : listeners( EventType.PERSIST_ONFLUSH ) ) {
listener.onPersist( event );
}
checkDelayedActionStatusAfterOperation();
checkNoUnresolvedActionsAfterOperation();
}
@ -807,11 +807,11 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
private Object fireMerge(MergeEvent event) {
errorIfClosed();
checkTransactionSynchStatus();
checkDelayedActionStatusBeforeOperation();
checkNoUnresolvedActionsBeforeOperation();
for ( MergeEventListener listener : listeners( EventType.MERGE ) ) {
listener.onMerge( event );
}
checkDelayedActionStatusAfterOperation();
checkNoUnresolvedActionsAfterOperation();
return event.getResult();
}

View File

@ -33,4 +33,5 @@ public class MultiPathCircleCascadeDelayedInsertTest extends MultiPathCircleCasc
"cascade/circle/MultiPathCircleCascadeDelayedInsert.hbm.xml"
};
}
}

View File

@ -34,6 +34,7 @@ import org.hibernate.TransientObjectException;
import org.hibernate.cfg.Configuration;
import org.hibernate.cfg.Environment;
import org.hibernate.engine.spi.SessionImplementor;
import org.hibernate.id.IdentityGenerator;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
import static org.junit.Assert.assertEquals;
@ -681,7 +682,7 @@ public class MultiPathCircleCascadeTest extends BaseCoreFunctionalTestCase {
}
}
else {
assertTrue( ex instanceof TransientObjectException );
assertTrue( ex instanceof PropertyValueException );
}
}