From ecbcc2d940f72f229c0fe10cd727c7e61b184a58 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Thu, 25 May 2023 12:46:10 +0200 Subject: [PATCH] HHH-16690 Fix re-saving for unloaded deletes --- .../internal/StatefulPersistenceContext.java | 11 ++++ .../org/hibernate/engine/spi/ActionQueue.java | 21 +++++++ .../engine/spi/PersistenceContext.java | 4 ++ .../engine/spi/SessionDelegatorBaseImpl.java | 5 ++ .../engine/spi/SessionImplementor.java | 4 ++ .../internal/AbstractSaveEventListener.java | 3 + .../internal/DefaultMergeEventListener.java | 16 ++++- .../hibernate/event/internal/EntityState.java | 13 ++++ .../org/hibernate/event/spi/EventSource.java | 5 ++ .../org/hibernate/internal/SessionImpl.java | 11 +++- .../DeleteUnloadedProxyTest.java | 59 ++++++++++++++++++- .../orm/test/deleteunloaded/ParentSub.java | 48 +++++++++++++++ 12 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/ParentSub.java diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java index 989acda49b..4db85b7950 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java @@ -1867,6 +1867,17 @@ public class StatefulPersistenceContext implements PersistenceContext { deletedUnloadedEntityKeys.add( key ); } + @Override + public void removeDeletedUnloadedEntityKey(EntityKey key) { + assert deletedUnloadedEntityKeys != null; + deletedUnloadedEntityKeys.remove( key ); + } + + @Override + public boolean containsDeletedUnloadedEntityKeys() { + return deletedUnloadedEntityKeys != null && !deletedUnloadedEntityKeys.isEmpty(); + } + @Override public int getCollectionEntriesSize() { return collectionEntries == null ? 0 : collectionEntries.size(); diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java index 64d1673101..662c09fe6e 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java @@ -50,6 +50,7 @@ import org.hibernate.internal.util.StringHelper; import org.hibernate.internal.util.collections.CollectionHelper; import org.hibernate.metamodel.mapping.PluralAttributeMapping; import org.hibernate.metamodel.mapping.internal.EntityCollectionPart; +import org.hibernate.persister.entity.EntityPersister; import org.hibernate.proxy.HibernateProxy; import org.hibernate.proxy.LazyInitializer; import org.hibernate.type.CollectionType; @@ -846,6 +847,26 @@ public class ActionQueue { || ( collectionCreations != null && !collectionCreations.isEmpty() ); } + public void unScheduleUnloadedDeletion(Object newEntity) { + final EntityPersister entityPersister = session.getEntityPersister( null, newEntity ); + final Object identifier = entityPersister.getIdentifier( newEntity, session ); + if ( deletions != null ) { + for ( int i = 0; i < deletions.size(); i++ ) { + EntityDeleteAction action = deletions.get( i ); + if ( action.getInstance() == null + && action.getEntityName().equals( entityPersister.getEntityName() ) + && entityPersister.getIdentifierMapping().areEqual( action.getId(), identifier, session ) ) { + session.getPersistenceContextInternal().removeDeletedUnloadedEntityKey( + session.generateEntityKey( identifier, entityPersister ) + ); + deletions.remove( i ); + return; + } + } + } + throw new AssertionFailure( "Unable to perform un-delete for unloaded entity delete " + entityPersister.getEntityName() ); + } + public void unScheduleDeletion(EntityEntry entry, Object rescuedEntity) { final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( rescuedEntity ); if ( lazyInitializer != null ) { diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/PersistenceContext.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/PersistenceContext.java index d3f96fedf7..2a227856b2 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/PersistenceContext.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/PersistenceContext.java @@ -757,6 +757,10 @@ public interface PersistenceContext { void registerDeletedUnloadedEntityKey(EntityKey key); + void removeDeletedUnloadedEntityKey(EntityKey key); + + boolean containsDeletedUnloadedEntityKeys(); + /** * The size of the internal map storing all collection entries. * (The map is not exposed directly, but the size is often useful) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java index e0d7763226..e5b7ae0ff2 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java @@ -1141,6 +1141,11 @@ public class SessionDelegatorBaseImpl implements SessionImplementor { delegate.forceFlush( e ); } + @Override + public void forceFlush(EntityKey e) throws HibernateException { + delegate.forceFlush( e ); + } + @Override public void merge(String entityName, Object object, MergeContext copiedAlready) throws HibernateException { delegate.merge( entityName, object, copiedAlready ); diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionImplementor.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionImplementor.java index 6884f42325..b8ba8353df 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionImplementor.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionImplementor.java @@ -87,6 +87,10 @@ public interface SessionImplementor extends Session, SharedSessionContractImplem * Initiate a flush to force deletion of a re-persisted entity. */ void forceFlush(EntityEntry e) throws HibernateException; + /** + * Initiate a flush to force deletion of a re-persisted entity. + */ + void forceFlush(EntityKey e) throws HibernateException; /** * Cascade the lock operation to the given child entity. diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java index 951f8019ed..6f83019961 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java @@ -203,6 +203,9 @@ public abstract class AbstractSaveEventListener throw new NonUniqueObjectException( id, persister.getEntityName() ); } } + else if ( persistenceContext.containsDeletedUnloadedEntityKey( key ) ) { + source.forceFlush( key ); + } persister.setIdentifier( entity, id, source ); return key; } diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultMergeEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultMergeEventListener.java index baced96f38..43d3113ecb 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultMergeEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultMergeEventListener.java @@ -156,6 +156,19 @@ public class DefaultMergeEventListener entityIsPersistent( event, copiedAlready ); break; default: //DELETED + if ( event.getSession().getPersistenceContext().getEntry( entity ) == null ) { + assert event.getSession().getPersistenceContext().containsDeletedUnloadedEntityKey( + event.getSession().generateEntityKey( + event.getSession() + .getEntityPersister( event.getEntityName(), entity ) + .getIdentifier( entity, event.getSession() ), + event.getSession().getEntityPersister( event.getEntityName(), entity ) + ) + ); + event.getSession().getActionQueue().unScheduleUnloadedDeletion( entity ); + entityIsDetached(event, copiedAlready); + break; + } throw new ObjectDeletedException( "deleted instance passed to merge", null, @@ -174,7 +187,8 @@ public class DefaultMergeEventListener EntityPersister persister = source.getEntityPersister( event.getEntityName(), entity ); Object id = persister.getIdentifier( entity, source ); if ( id != null ) { - final Object managedEntity = persistenceContext.getEntity( source.generateEntityKey( id, persister ) ); + final EntityKey entityKey = source.generateEntityKey( id, persister ); + final Object managedEntity = persistenceContext.getEntity( entityKey ); entry = persistenceContext.getEntry( managedEntity ); if ( entry != null ) { // we have a special case of a detached entity from the diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/EntityState.java b/hibernate-core/src/main/java/org/hibernate/event/internal/EntityState.java index a0f537827f..57834fd57c 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/EntityState.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/EntityState.java @@ -8,10 +8,13 @@ package org.hibernate.event.internal; import org.hibernate.engine.internal.ForeignKeys; import org.hibernate.engine.spi.EntityEntry; +import org.hibernate.engine.spi.EntityKey; +import org.hibernate.engine.spi.PersistenceContext; import org.hibernate.engine.spi.SessionImplementor; import org.hibernate.engine.spi.Status; import org.hibernate.internal.CoreLogging; import org.hibernate.internal.CoreMessageLogger; +import org.hibernate.persister.entity.EntityPersister; public enum EntityState { PERSISTENT, TRANSIENT, DETACHED, DELETED; @@ -65,6 +68,16 @@ public enum EntityState { if ( LOG.isTraceEnabled() ) { LOG.tracev( "Detached instance of: {0}", EventUtil.getLoggableName( entityName, entity ) ); } + + final PersistenceContext persistenceContext = source.getPersistenceContextInternal(); + if ( persistenceContext.containsDeletedUnloadedEntityKeys() ) { + final EntityPersister entityPersister = source.getEntityPersister( entityName, entity ); + final Object identifier = entityPersister.getIdentifier( entity, source ); + final EntityKey entityKey = source.generateEntityKey( identifier, entityPersister ); + if ( persistenceContext.containsDeletedUnloadedEntityKey( entityKey ) ) { + return EntityState.DELETED; + } + } return DETACHED; } diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/EventSource.java b/hibernate-core/src/main/java/org/hibernate/event/spi/EventSource.java index b35fe59b04..c35a55da4d 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/spi/EventSource.java +++ b/hibernate-core/src/main/java/org/hibernate/event/spi/EventSource.java @@ -9,6 +9,7 @@ package org.hibernate.event.spi; import org.hibernate.HibernateException; import org.hibernate.engine.spi.ActionQueue; import org.hibernate.engine.spi.EntityEntry; +import org.hibernate.engine.spi.EntityKey; import org.hibernate.engine.spi.SessionImplementor; import org.hibernate.persister.entity.EntityPersister; @@ -32,6 +33,10 @@ public interface EventSource extends SessionImplementor { * Force an immediate flush */ void forceFlush(EntityEntry e) throws HibernateException; + /** + * Force an immediate flush + */ + void forceFlush(EntityKey e) throws HibernateException; /** * Cascade merge an entity instance diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java index cc9dc085ae..426c171681 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -1427,18 +1427,23 @@ public class SessionImpl @Override public void forceFlush(EntityEntry entityEntry) throws HibernateException { + forceFlush( entityEntry.getEntityKey() ); + } + + @Override + public void forceFlush(EntityKey key) throws HibernateException { if ( log.isDebugEnabled() ) { log.debugf( "Flushing to force deletion of re-saved object: %s", - MessageHelper.infoString( entityEntry.getPersister(), entityEntry.getId(), getFactory() ) + MessageHelper.infoString( key.getPersister(), key.getIdentifier(), getFactory() ) ); } if ( persistenceContext.getCascadeLevel() > 0 ) { throw new ObjectDeletedException( "deleted object would be re-saved by cascade (remove deleted object from associations)", - entityEntry.getId(), - entityEntry.getPersister().getEntityName() + key.getIdentifier(), + key.getPersister().getEntityName() ); } checkOpenOrWaitingForAutoClose(); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java index 22d6c190d8..afcb8e3507 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/DeleteUnloadedProxyTest.java @@ -2,15 +2,19 @@ package org.hibernate.orm.test.deleteunloaded; import org.hibernate.Transaction; import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import static org.hibernate.Hibernate.isInitialized; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -@DomainModel( annotatedClasses = { Parent.class, Child.class } ) +@DomainModel( annotatedClasses = { Parent.class, Child.class, ParentSub.class } ) @SessionFactory //@ServiceRegistry( // settings = { @@ -18,6 +22,15 @@ import static org.junit.jupiter.api.Assertions.assertNull; // } //) public class DeleteUnloadedProxyTest { + + @AfterEach + public void cleanup(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.createMutationQuery( "delete from ParentSub" ).executeUpdate(); + session.createMutationQuery( "delete from Child" ).executeUpdate(); + session.createMutationQuery( "delete from Parent" ).executeUpdate(); + } ); + } @Test public void testAttached(SessionFactoryScope scope) { Parent p = new Parent(); @@ -86,4 +99,48 @@ public class DeleteUnloadedProxyTest { assertNull( em.find( Child.class, c.getId() ) ); } ); } + + @Test + @JiraKey( "HHH-16690" ) + public void testRePersist(SessionFactoryScope scope) { + Parent p = new Parent(); + ParentSub ps = new ParentSub( 1L, "abc", p ); + scope.inTransaction( em -> { + em.persist( p ); + em.persist( ps ); + } ); + scope.inTransaction( em -> { + ParentSub sub = em.getReference( ParentSub.class, 1L ); + assertFalse( isInitialized( sub ) ); + em.remove( sub ); + em.persist( new ParentSub( 1L, "def", p ) ); + } ); + scope.inSession( em -> { + ParentSub sub = em.find( ParentSub.class, 1L ); + assertNotNull( sub ); + assertEquals( "def", sub.getData() ); + } ); + } + + @Test + @JiraKey( "HHH-16690" ) + public void testReMerge(SessionFactoryScope scope) { + Parent p = new Parent(); + ParentSub ps = new ParentSub( 1L, "abc", p ); + scope.inTransaction( em -> { + em.persist( p ); + em.persist( ps ); + } ); + scope.inTransaction( em -> { + ParentSub sub = em.getReference( ParentSub.class, 1L ); + assertFalse( isInitialized( sub ) ); + em.remove( sub ); + em.merge( new ParentSub( 1L, "def", p ) ); + } ); + scope.inSession( em -> { + ParentSub sub = em.find( ParentSub.class, 1L ); + assertNotNull( sub ); + assertEquals( "def", sub.getData() ); + } ); + } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/ParentSub.java b/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/ParentSub.java new file mode 100644 index 0000000000..f93f0f8393 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/deleteunloaded/ParentSub.java @@ -0,0 +1,48 @@ +package org.hibernate.orm.test.deleteunloaded; + +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import jakarta.persistence.OneToOne; + +@Entity +public class ParentSub { + @Id + private long id; + private String data; + @OneToOne(fetch = FetchType.LAZY) + private Parent parent; + + public ParentSub() { + } + + public ParentSub(long id, String data, Parent parent) { + this.id = id; + this.data = data; + this.parent = parent; + } + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public String getData() { + return data; + } + + public void setData(String data) { + this.data = data; + } + + public Parent getParent() { + return parent; + } + + public void setParent(Parent parent) { + this.parent = parent; + } +}