From 8841996984ab9e1a2e170e9fac7ad9b6392948a6 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 22 Nov 2025 19:59:05 +0100 Subject: [PATCH 1/2] introduce DetachedObjectException and remove obsolete code relating to reassociation from lock() and refresh() also remove code for reattaching proxies during flush --- .../hibernate/DetachedObjectException.java | 20 ++++ .../spi/AbstractPersistentCollection.java | 38 +++--- .../hibernate/engine/internal/Cascade.java | 3 +- .../internal/StatefulPersistenceContext.java | 99 +++++++++++++--- .../engine/spi/PersistenceContext.java | 6 + .../internal/DefaultDeleteEventListener.java | 30 ++--- .../internal/DefaultEvictEventListener.java | 3 +- .../internal/DefaultLockEventListener.java | 110 ++---------------- .../internal/DefaultMergeEventListener.java | 9 +- .../internal/DefaultPersistEventListener.java | 17 ++- .../internal/DefaultRefreshEventListener.java | 36 ++---- .../event/internal/OnLockVisitor.java | 68 ----------- .../event/internal/OnReplicateVisitor.java | 4 +- .../event/internal/OnUpdateVisitor.java | 15 ++- .../event/internal/ProxyVisitor.java | 16 +-- .../event/internal/ReattachVisitor.java | 15 ++- .../hibernate/event/internal/WrapVisitor.java | 2 +- .../hibernate/internal/CoreMessageLogger.java | 4 + .../internal/ExceptionConverterImpl.java | 11 ++ .../org/hibernate/internal/SessionImpl.java | 15 +-- .../AbstractEntityWithManyToManyTest.java | 6 - .../hibernate/orm/test/jpa/lock/LockTest.java | 7 +- .../orm/test/jpa/ops/PersistTest.java | 4 +- .../DetachedEntityManagerArgumentsTest.java | 2 +- .../DetachedSessionArgumentsTest.java | 2 +- 25 files changed, 238 insertions(+), 304 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/DetachedObjectException.java delete mode 100644 hibernate-core/src/main/java/org/hibernate/event/internal/OnLockVisitor.java diff --git a/hibernate-core/src/main/java/org/hibernate/DetachedObjectException.java b/hibernate-core/src/main/java/org/hibernate/DetachedObjectException.java new file mode 100644 index 000000000000..c177ade998c0 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/DetachedObjectException.java @@ -0,0 +1,20 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate; + +/** + * Thrown if a detached instance of an entity class is passed to + * a {@link Session} method that expects a managed instance. + * + * @author Gavin King + * + * @since 7.0 + */ +@Incubating +public class DetachedObjectException extends HibernateException { + public DetachedObjectException(String message) { + super( message ); + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java b/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java index d657c9d2cfbe..f813dc7b65e9 100644 --- a/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java +++ b/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java @@ -7,7 +7,6 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -22,8 +21,6 @@ import org.hibernate.HibernateException; import org.hibernate.LazyInitializationException; import org.hibernate.engine.spi.CollectionEntry; -import org.hibernate.engine.spi.EntityEntry; -import org.hibernate.engine.spi.PersistenceContext; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.engine.spi.Status; import org.hibernate.engine.spi.TypedValue; @@ -37,6 +34,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; +import static java.util.Collections.emptyIterator; import static java.util.Collections.emptyList; import static org.hibernate.collection.internal.CollectionLogger.COLLECTION_LOGGER; import static org.hibernate.engine.internal.ForeignKeys.getEntityIdentifier; @@ -686,7 +684,8 @@ public final boolean unsetSession(SharedSessionContractImplementor currentSessio if ( allowLoadOutsideTransaction && !initialized && session.getLoadQueryInfluencers().hasEnabledFilters() ) { - COLLECTION_LOGGER.enabledFiltersWhenDetachFromSession( collectionInfoString( getRole(), getKey() ) ); + COLLECTION_LOGGER.enabledFiltersWhenDetachFromSession( + collectionInfoString( getRole(), getKey() ) ); } session = null; } @@ -694,7 +693,8 @@ public final boolean unsetSession(SharedSessionContractImplementor currentSessio } else { if ( session != null ) { - COLLECTION_LOGGER.logCannotUnsetUnexpectedSessionInCollection( unexpectedSessionStateMessage( currentSession ) ); + COLLECTION_LOGGER.logCannotUnsetUnexpectedSessionInCollection( + unexpectedSessionStateMessage( currentSession ) ); } return false; } @@ -704,20 +704,23 @@ private void logDiscardedQueuedOperations() { try { if ( wasTransactionRolledBack() ) { // It was due to a rollback. - if ( COLLECTION_LOGGER.isDebugEnabled()) { - COLLECTION_LOGGER.queuedOperationWhenDetachFromSessionOnRollback( collectionInfoString( getRole(), getKey() ) ); + if ( COLLECTION_LOGGER.isDebugEnabled() ) { + COLLECTION_LOGGER.queuedOperationWhenDetachFromSessionOnRollback( + collectionInfoString( getRole(), getKey() ) ); } } else { // We don't know why the collection is being detached. // Just log the info. - COLLECTION_LOGGER.queuedOperationWhenDetachFromSession( collectionInfoString( getRole(), getKey() ) ); + COLLECTION_LOGGER.queuedOperationWhenDetachFromSession( + collectionInfoString( getRole(), getKey() ) ); } } catch (Exception e) { // We don't know why the collection is being detached. // Just log the info. - COLLECTION_LOGGER.queuedOperationWhenDetachFromSession( collectionInfoString( getRole(), getKey() ) ); + COLLECTION_LOGGER.queuedOperationWhenDetachFromSession( + collectionInfoString( getRole(), getKey() ) ); } } @@ -756,7 +759,8 @@ else if ( this.session != null ) { } } if ( hasQueuedOperations() ) { - COLLECTION_LOGGER.queuedOperationWhenAttachToSession( collectionInfoString( getRole(), getKey() ) ); + COLLECTION_LOGGER.queuedOperationWhenAttachToSession( + collectionInfoString( getRole(), getKey() ) ); } this.session = session; return true; @@ -872,7 +876,7 @@ public void remove() { }; } else { - return Collections.emptyIterator(); + return emptyIterator(); } } @@ -1070,8 +1074,8 @@ public A[] toArray(A[] array) { public final boolean equals(Object object) { return object == this || object instanceof Set that - && that.size() == this.size() - && containsAll( that ); + && that.size() == this.size() + && containsAll( that ); } @Override @@ -1306,11 +1310,11 @@ protected static Collection getOrphans( // collect EntityIdentifier(s) of the *current* elements - add them into a HashSet for fast access final java.util.Set currentIds = new HashSet<>(); final java.util.Set currentSaving = new IdentitySet<>(); - final PersistenceContext persistenceContext = session.getPersistenceContextInternal(); + final var persistenceContext = session.getPersistenceContextInternal(); for ( Object current : currentElements ) { if ( current != null && isNotTransient( entityName, current, null, session ) ) { - final EntityEntry ee = persistenceContext.getEntry( current ); - if ( ee != null && ee.getStatus() == Status.SAVING ) { + final var entityEntry = persistenceContext.getEntry( current ); + if ( entityEntry != null && entityEntry.getStatus() == Status.SAVING ) { currentSaving.add( current ); } else { @@ -1335,7 +1339,7 @@ protected static Collection getOrphans( private static boolean mayUseIdDirect(Type idType) { if ( idType instanceof BasicType basicType ) { - final Class javaType = basicType.getJavaType(); + final var javaType = basicType.getJavaType(); return javaType == String.class || javaType == Integer.class || javaType == Long.class diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java index d490df522cc4..cac112458f8e 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java @@ -322,8 +322,9 @@ private static void cascadeLogicalOneToOneOrphanRemoval( if ( child == null || loadedValue != null && child != loadedValue ) { EntityEntry valueEntry = persistenceContext.getEntry( loadedValue ); if ( valueEntry == null && isHibernateProxy( loadedValue ) ) { - // un-proxy and re-associate for cascade operation + // unproxy and reassociate for cascade operation // useful for @OneToOne defined as FetchType.LAZY + //TODO: what should really happen here??? loadedValue = persistenceContext.unproxyAndReassociate( loadedValue ); valueEntry = persistenceContext.getEntry( loadedValue ); // HHH-11965 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 1470d7afd928..c4335d0a76e7 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 @@ -23,6 +23,7 @@ import java.util.function.Supplier; import org.hibernate.AssertionFailure; +import org.hibernate.DetachedObjectException; import org.hibernate.Hibernate; import org.hibernate.HibernateException; import org.hibernate.LockMode; @@ -691,26 +692,62 @@ public boolean containsProxy(Object entity) { @Override public boolean reassociateIfUninitializedProxy(Object value) throws MappingException { - if ( !Hibernate.isInitialized( value ) ) { - // could be a proxy - final var lazyInitializer = extractLazyInitializer( value ); - if ( lazyInitializer != null ) { + if ( Hibernate.isInitialized( value ) ) { + return false; + } + // could be a proxy + final var lazyInitializer = extractLazyInitializer( value ); + if ( lazyInitializer != null ) { + final boolean uninitialized = lazyInitializer.isUninitialized(); + if ( uninitialized ) { reassociateProxy( lazyInitializer, asHibernateProxy( value ) ); - return true; } - // or an uninitialized enhanced entity ("bytecode proxy") - if ( isPersistentAttributeInterceptable( value ) ) { - final var bytecodeProxy = asPersistentAttributeInterceptable( value ); - final var interceptor = - (BytecodeLazyAttributeInterceptor) - bytecodeProxy.$$_hibernate_getInterceptor(); - if ( interceptor != null ) { - interceptor.setSession( getSession() ); - } - return true; + return uninitialized; + } + // or an uninitialized enhanced entity ("bytecode proxy") + else if ( isPersistentAttributeInterceptable( value ) ) { + final var interceptor = + (BytecodeLazyAttributeInterceptor) + asPersistentAttributeInterceptable( value ) + .$$_hibernate_getInterceptor(); + final boolean uninitialized = + interceptor instanceof EnhancementAsProxyLazinessInterceptor enhancementInterceptor + && !enhancementInterceptor.isInitialized(); + if ( uninitialized ) { + interceptor.setSession( getSession() ); } + return uninitialized; + } + else { + return false; + } + } + + @Override + public boolean isUninitializedProxy(Object value) throws MappingException { + // could be a proxy + final var lazyInitializer = extractLazyInitializer( value ); + if ( lazyInitializer != null ) { + if ( lazyInitializer.getSession() != session ) { + throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); + } + return lazyInitializer.isUninitialized(); + } + // or an uninitialized enhanced entity ("bytecode proxy") + else if ( isPersistentAttributeInterceptable( value ) ) { + final var interceptor = + (BytecodeLazyAttributeInterceptor) + asPersistentAttributeInterceptable( value ) + .$$_hibernate_getInterceptor(); + if ( interceptor != null && interceptor.getLinkedSession() != session ) { + throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); + } + return interceptor instanceof EnhancementAsProxyLazinessInterceptor enhancementInterceptor + && !enhancementInterceptor.isInitialized(); + } + else { + return false; } - return false; } @Override @@ -755,7 +792,7 @@ public Object unproxy(Object maybeProxy) throws HibernateException { final var lazyInitializer = extractLazyInitializer( maybeProxy ); if ( lazyInitializer != null ) { if ( lazyInitializer.isUninitialized() ) { - throw new PersistentObjectException( "object was an uninitialized proxy for " + throw new PersistentObjectException( "Object was an uninitialized proxy for " + lazyInitializer.getEntityName() ); } //unwrap the object and return @@ -766,6 +803,34 @@ public Object unproxy(Object maybeProxy) throws HibernateException { } } + @Override + public Object unproxyLoadingIfNecessary(final Object maybeProxy) throws HibernateException { + final var lazyInitializer = extractLazyInitializer( maybeProxy ); + if ( lazyInitializer != null ) { + if ( lazyInitializer.getSession() != session ) { + throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); + } + //initialize + unwrap the object and return it + return lazyInitializer.getImplementation(); + } + else if ( isPersistentAttributeInterceptable( maybeProxy ) ) { + final var interceptor = + asPersistentAttributeInterceptable( maybeProxy ) + .$$_hibernate_getInterceptor(); + if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor lazinessInterceptor ) { + if ( lazinessInterceptor.getLinkedSession() != session ) { + throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); + } + lazinessInterceptor.forceInitialize( maybeProxy, null ); + } + return maybeProxy; + } + else { + return maybeProxy; + } + } + + @Override public Object unproxyAndReassociate(final Object maybeProxy) throws HibernateException { final var lazyInitializer = extractLazyInitializer( maybeProxy ); 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 881dba6098a0..0f5f509018e2 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 @@ -296,6 +296,12 @@ EntityEntry addReferenceEntry( */ Object unproxy(Object maybeProxy); + @Incubating + Object unproxyLoadingIfNecessary(final Object maybeProxy); + + @Incubating + boolean isUninitializedProxy(Object value); + /** * Possibly unproxy the given reference and reassociate it with the current session. * diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java index ee51e8b9ef2f..44c3fae80431 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java @@ -5,6 +5,7 @@ package org.hibernate.event.internal; import org.hibernate.CacheMode; +import org.hibernate.DetachedObjectException; import org.hibernate.HibernateException; import org.hibernate.LockMode; import org.hibernate.StaleObjectStateException; @@ -166,7 +167,7 @@ private void deleteUnmanagedInstance(DeleteEvent event, DeleteContext transientE private void deleteDetachedEntity( DeleteEvent event, DeleteContext transientEntities, Object entity, EntityPersister persister, EventSource source) { if ( source.getFactory().getSessionFactoryOptions().isJpaBootstrap() ) { - throw new IllegalArgumentException( "Given entity is not associated with the persistence context" ); + throw new DetachedObjectException( "Given entity is not associated with the persistence context" ); } final Object id = persister.getIdentifier( entity, source ); if ( id == null ) { @@ -182,18 +183,19 @@ private void deleteDetachedEntity( new OnUpdateVisitor( source, id, entity ).process( entity, persister ); - final var persistenceContext = source.getPersistenceContextInternal(); - final var entityEntry = persistenceContext.addEntity( - entity, - persister.isMutable() ? Status.MANAGED : Status.READ_ONLY, - persister.getValues( entity ), - key, - version, - LockMode.NONE, - true, - persister, - false - ); + final var entityEntry = + source.getPersistenceContextInternal() + .addEntity( + entity, + persister.isMutable() ? Status.MANAGED : Status.READ_ONLY, + persister.getValues( entity ), + key, + version, + LockMode.NONE, + true, + persister, + false + ); persister.afterReassociate( entity, source ); delete( event, transientEntities, source, entity, persister, id, version, entityEntry ); @@ -524,7 +526,7 @@ protected void cascadeAfterDelete( final var persistenceContext = session.getPersistenceContextInternal(); persistenceContext.incrementCascadeLevel(); try { - // cascade-delete to many-to-one AFTER the parent was deleted + // cascade delete to many-to-one AFTER the parent was deleted Cascade.cascade( CascadingActions.REMOVE, CascadePoint.BEFORE_INSERT_AFTER_DELETE, diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultEvictEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultEvictEventListener.java index 515d64532f36..f0195dc11662 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultEvictEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultEvictEventListener.java @@ -13,7 +13,6 @@ import org.hibernate.event.spi.EvictEvent; import org.hibernate.event.spi.EvictEventListener; import org.hibernate.persister.entity.EntityPersister; -import org.hibernate.proxy.LazyInitializer; import static org.hibernate.event.internal.EventListenerLogging.EVENT_LISTENER_LOGGER; import static org.hibernate.pretty.MessageHelper.infoString; @@ -43,7 +42,7 @@ public void onEvict(EvictEvent event) throws HibernateException { if ( object == null ) { throw new NullPointerException( "null passed to Session.evict()" ); } - final LazyInitializer lazyInitializer = extractLazyInitializer( object ); + final var lazyInitializer = extractLazyInitializer( object ); if ( lazyInitializer != null ) { final Object id = lazyInitializer.getInternalIdentifier(); if ( id == null ) { diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java index 3c8eec03629c..20f99c16e615 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java @@ -4,27 +4,15 @@ */ package org.hibernate.event.internal; +import org.hibernate.DetachedObjectException; import org.hibernate.HibernateException; import org.hibernate.LockMode; -import org.hibernate.TransientObjectException; -import org.hibernate.engine.internal.Cascade; -import org.hibernate.engine.internal.CascadePoint; -import org.hibernate.engine.internal.ForeignKeys; -import org.hibernate.engine.spi.CascadingActions; -import org.hibernate.engine.spi.EntityEntry; -import org.hibernate.engine.spi.Status; -import org.hibernate.event.spi.AbstractSessionEvent; import org.hibernate.event.spi.LockEvent; import org.hibernate.event.spi.LockEventListener; -import org.hibernate.persister.entity.EntityPersister; -import org.hibernate.type.TypeHelper; -import static org.hibernate.engine.internal.Versioning.getVersion; -import static org.hibernate.event.internal.EventListenerLogging.EVENT_LISTENER_LOGGER; import static org.hibernate.loader.ast.internal.LoaderHelper.upgradeLock; -import static org.hibernate.pretty.MessageHelper.infoString; /** * Defines the default lock event listeners used by hibernate to lock entities @@ -42,105 +30,27 @@ public class DefaultLockEventListener implements LockEventListener { @Override public void onLock(LockEvent event) throws HibernateException { - if ( event.getObject() == null ) { - throw new NullPointerException( "attempted to lock null" ); + final Object instance = event.getObject(); + if ( instance == null ) { + throw new NullPointerException( "Attempted to lock null" ); } final var lockMode = event.getLockMode(); if ( lockMode == LockMode.WRITE || lockMode == LockMode.UPGRADE_SKIPLOCKED ) { - throw new HibernateException( "Invalid lock mode for lock()" ); + throw new IllegalArgumentException( "Invalid lock mode '" + lockMode + "' passed to 'lock()'" ); } final var source = event.getSession(); final var persistenceContext = source.getPersistenceContextInternal(); - final Object entity = persistenceContext.unproxyAndReassociate( event.getObject() ); - //TODO: if object was an uninitialized proxy, this is inefficient, + final Object entity = persistenceContext.unproxyLoadingIfNecessary( instance ); + //TODO: if instance was an uninitialized proxy, this is inefficient, // resulting in two SQL selects - var entry = persistenceContext.getEntry( entity ); - if ( entry == null ) { - final var persister = source.getEntityPersister( event.getEntityName(), entity ); - final Object id = persister.getIdentifier( entity, source ); - if ( !ForeignKeys.isNotTransient( event.getEntityName(), entity, Boolean.FALSE, source ) ) { - throw new TransientObjectException( "Cannot lock unsaved transient instance of entity '" - + persister.getEntityName() + "'" ); - } - entry = reassociate( event, entity, id, persister ); - cascadeOnLock( event, persister, entity ); + final var entry = persistenceContext.getEntry( entity ); + if ( entry == null && instance == entity ) { + throw new DetachedObjectException( "Given entity is not associated with the persistence context" ); } upgradeLock( entity, entry, event.getLockOptions(), event.getSession() ); } - - private void cascadeOnLock(LockEvent event, EntityPersister persister, Object entity) { - final var source = event.getSession(); - final var persistenceContext = source.getPersistenceContextInternal(); - persistenceContext.incrementCascadeLevel(); - try { - Cascade.cascade( - CascadingActions.LOCK, - CascadePoint.AFTER_LOCK, - source, - persister, - entity, - event.getLockOptions() - ); - } - finally { - persistenceContext.decrementCascadeLevel(); - } - } - - /** - * Associates a given entity (either transient or associated with another session) - * to the given session. - * - * @param event The event triggering the re-association - * @param object The entity to be associated - * @param id The id of the entity. - * @param persister The entity's persister instance. - * - * @return An EntityEntry representing the entity within this session. - */ - protected final EntityEntry reassociate(AbstractSessionEvent event, Object object, Object id, EntityPersister persister) { - - if ( EVENT_LISTENER_LOGGER.isTraceEnabled() ) { - EVENT_LISTENER_LOGGER.reassociatingTransientInstance( - infoString( persister, id, event.getFactory() ) ); - } - - final var source = event.getSession(); - final var key = source.generateEntityKey( id, persister ); - final var persistenceContext = source.getPersistenceContext(); - - persistenceContext.checkUniqueness( key, object ); - - //get a snapshot - final Object[] values = persister.getValues( object ); - TypeHelper.deepCopy( - values, - persister.getPropertyTypes(), - persister.getPropertyUpdateability(), - values, - source - ); - - final var newEntry = persistenceContext.addEntity( - object, - persister.isMutable() ? Status.MANAGED : Status.READ_ONLY, - values, - key, - getVersion( values, persister ), - LockMode.NONE, - true, - persister, - false - ); - - new OnLockVisitor( source, id, object ).process( object, persister ); - - persister.afterReassociate( object, source ); - - return newEntry; - } } 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 0d43b7dbf5b1..6e89f91e6353 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 @@ -362,11 +362,12 @@ private static class CollectionVisitor extends WrapVisitor { @Override protected Object processCollection(Object collection, CollectionType collectionType) { if ( collection instanceof PersistentCollection persistentCollection ) { + final var session = getSession(); final var persister = - getSession().getFactory().getMappingMetamodel() + session.getFactory().getMappingMetamodel() .getCollectionDescriptor( collectionType.getRole() ); final var collectionEntry = - getSession().getPersistenceContextInternal() + session.getPersistenceContextInternal() .getCollectionEntry( persistentCollection ); if ( !persistentCollection.equalsSnapshot( persister ) ) { collectionEntry.resetStoredSnapshot( persistentCollection, persistentCollection.getSnapshot( persister ) ); @@ -374,10 +375,6 @@ protected Object processCollection(Object collection, CollectionType collectionT } return null; } - @Override - Object processEntity(Object value, EntityType entityType) { - return null; - } } private void saveTransientEntity( diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPersistEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPersistEventListener.java index 3f8d9ecb7d0e..2c044d8f058a 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPersistEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPersistEventListener.java @@ -20,6 +20,7 @@ import static org.hibernate.event.internal.EntityState.getEntityState; import static org.hibernate.event.internal.EventListenerLogging.EVENT_LISTENER_LOGGER; +import static org.hibernate.event.internal.EventUtil.getLoggableName; import static org.hibernate.pretty.MessageHelper.infoString; import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer; @@ -62,7 +63,7 @@ public void onPersist(PersistEvent event, PersistContext createCache) throws Hib if ( lazyInitializer != null ) { if ( lazyInitializer.isUninitialized() ) { if ( lazyInitializer.getSession() != event.getSession() ) { - throw new PersistentObjectException( "uninitialized proxy passed to persist()" ); + throw new PersistentObjectException( "Uninitialized proxy passed to persist()" ); } } else { @@ -81,7 +82,7 @@ private void persist(PersistEvent event, PersistContext createCache, Object enti switch ( getEntityState( entity, entityName, entityEntry, source, true ) ) { case DETACHED: throw new PersistentObjectException( "Detached entity passed to persist: " - + EventUtil.getLoggableName( event.getEntityName(), entity) ); + + getLoggableName( event.getEntityName(), entity) ); case PERSISTENT: entityIsPersistent( event, createCache ); break; @@ -95,17 +96,15 @@ private void persist(PersistEvent event, PersistContext createCache, Object enti entityIsDeleted( event, createCache ); break; default: - throw new ObjectDeletedException( - "Deleted entity passed to persist", - null, - EventUtil.getLoggableName( event.getEntityName(), entity ) - ); + throw new ObjectDeletedException( "Deleted entity passed to persist", null, + getLoggableName( entityName, entity ) ); } } private static String entityName(PersistEvent event, Object entity, EntityEntry entityEntry) { - if ( event.getEntityName() != null ) { - return event.getEntityName(); + final String explicitEntityName = event.getEntityName(); + if ( explicitEntityName != null ) { + return explicitEntityName; } else { // changes event.entityName by side effect! diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java index 0fe45834b391..0fde36a8b0ad 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java @@ -4,11 +4,10 @@ */ package org.hibernate.event.internal; +import org.hibernate.DetachedObjectException; import org.hibernate.HibernateException; import org.hibernate.LockMode; import org.hibernate.LockOptions; -import org.hibernate.NonUniqueObjectException; -import org.hibernate.TransientObjectException; import org.hibernate.UnresolvableObjectException; import org.hibernate.cache.spi.access.SoftLock; import org.hibernate.engine.internal.Cascade; @@ -54,11 +53,14 @@ public void onRefresh(RefreshEvent event, RefreshContext refreshedAlready) { final var source = event.getSession(); final var persistenceContext = source.getPersistenceContextInternal(); final Object object = event.getObject(); - if ( persistenceContext.reassociateIfUninitializedProxy( object ) ) { + if ( object == null ) { + throw new NullPointerException( "Attempted to refresh null" ); + } + else if ( persistenceContext.isUninitializedProxy( object ) ) { handleUninitializedProxy( event, refreshedAlready, source, object, persistenceContext ); } else { - final Object entity = persistenceContext.unproxyAndReassociate( object ); + final Object entity = persistenceContext.unproxyLoadingIfNecessary( object ); if ( refreshedAlready.add( entity) ) { refresh( event, refreshedAlready, entity ); } @@ -125,34 +127,18 @@ private static void refresh(RefreshEvent event, RefreshContext refreshedAlready, final EntityPersister persister; final Object id; if ( entry == null ) { - //refresh() does not pass an entityName - persister = source.getEntityPersister( event.getEntityName(), object ); - id = persister.getIdentifier( object, event.getSession() ); - if ( id == null ) { - throw new TransientObjectException( "Cannot refresh instance of entity '" + persister.getEntityName() - + "' because it has a null identifier" ); - } - if ( EVENT_LISTENER_LOGGER.isTraceEnabled() ) { - EVENT_LISTENER_LOGGER.refreshingTransient( - infoString( persister, id, event.getFactory() ) ); - } - if ( persistenceContext.getEntry( source.generateEntityKey( id, persister ) ) != null ) { - throw new NonUniqueObjectException( id, persister.getEntityName() ); - } + throw new DetachedObjectException( "Given entity is not associated with the persistence context" ); } else { + persister = entry.getPersister(); + id = entry.getId(); if ( EVENT_LISTENER_LOGGER.isTraceEnabled() ) { EVENT_LISTENER_LOGGER.refreshing( - infoString( entry.getPersister(), entry.getId(), event.getFactory() ) ); + infoString( persister, id, event.getFactory() ) ); } if ( !entry.isExistsInDatabase() ) { - throw new UnresolvableObjectException( - entry.getId(), - "this instance does not yet exist as a row in the database" - ); + throw new UnresolvableObjectException( id, persister.getEntityName() ); } - persister = entry.getPersister(); - id = entry.getId(); } // cascade the refresh prior to refreshing this entity diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/OnLockVisitor.java b/hibernate-core/src/main/java/org/hibernate/event/internal/OnLockVisitor.java deleted file mode 100644 index e9ae9db88778..000000000000 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/OnLockVisitor.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.event.internal; - -import org.hibernate.HibernateException; -import org.hibernate.collection.spi.PersistentCollection; -import org.hibernate.event.spi.EventSource; -import org.hibernate.type.CollectionType; - -/** - * When a transient entity is passed to lock(), we must inspect all its collections and - *
    - *
  1. associate any uninitialized PersistentCollections with this session - *
  2. associate any initialized PersistentCollections with this session, using the - * existing snapshot - *
  3. throw an exception for each "new" collection - *
- * - * @author Gavin King - */ -public class OnLockVisitor extends ReattachVisitor { - - public OnLockVisitor(EventSource session, Object key, Object owner) { - super( session, key, owner ); - } - - @Override - public Object processCollection(Object collection, CollectionType type) throws HibernateException { - if ( collection == null ) { - return null; - } - - final var session = getSession(); - final var persister = - session.getFactory().getMappingMetamodel() - .getCollectionDescriptor( type.getRole() ); - if ( collection instanceof PersistentCollection persistentCollection ) { - if ( persistentCollection.setCurrentSession( session ) ) { - if ( isOwnerUnchanged( persister, extractCollectionKeyFromOwner( persister ), persistentCollection ) ) { - // a "detached" collection that originally belonged to the same entity - if ( persistentCollection.isDirty() ) { - throw new HibernateException( "reassociated object has dirty collection" ); - } - reattachCollection( persistentCollection, type ); - } - else { - // a "detached" collection that belonged to a different entity - throw new HibernateException( "reassociated object has dirty collection reference" ); - } - } - else { - // a collection loaded in the current session - // cannot possibly be the collection belonging - // to the entity passed to update() - throw new HibernateException( "reassociated object has dirty collection reference" ); - } - } - else { - // brand new collection - //TODO: or an array!! we can't lock objects with arrays now?? - throw new HibernateException( "reassociated object has dirty collection reference (or an array)" ); - } - return null; - } - -} diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/OnReplicateVisitor.java b/hibernate-core/src/main/java/org/hibernate/event/internal/OnReplicateVisitor.java index a29866e9b476..a25578077b31 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/OnReplicateVisitor.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/OnReplicateVisitor.java @@ -10,8 +10,8 @@ import org.hibernate.type.CollectionType; /** - * When an entity is passed to replicate(), and there is an existing row, we must - * inspect all its collections and + * When an entity is passed to {@link org.hibernate.Session#replicate}, and there + * is an existing row, we must inspect all its collections and: *
    *
  1. associate any uninitialized PersistentCollections with this session *
  2. associate any initialized PersistentCollections with this session, using the diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/OnUpdateVisitor.java b/hibernate-core/src/main/java/org/hibernate/event/internal/OnUpdateVisitor.java index 4e083feb66e5..6a716d7cf522 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/OnUpdateVisitor.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/OnUpdateVisitor.java @@ -10,12 +10,15 @@ import org.hibernate.type.CollectionType; /** - * When an entity is passed to update(), we must inspect all its collections and - * 1. associate any uninitialized PersistentCollections with this session - * 2. associate any initialized PersistentCollections with this session, using the - * existing snapshot - * 3. execute a collection removal (SQL DELETE) for each null collection property - * or "new" collection + * When a detached entity is passed to {@link org.hibernate.Session#remove(Object)}, + * we must inspect all its collections and: + *
      + *
    1. associate any uninitialized PersistentCollections with this session + *
    2. associate any initialized PersistentCollections with this session, using the + * existing snapshot + *
    3. execute a collection removal (SQL DELETE) for each null collection property + * or "new" collection + *
    * * @author Gavin King */ diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/ProxyVisitor.java b/hibernate-core/src/main/java/org/hibernate/event/internal/ProxyVisitor.java index 7d08753bb5d4..d0e22cde10de 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/ProxyVisitor.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/ProxyVisitor.java @@ -9,10 +9,11 @@ import org.hibernate.event.spi.EventSource; import org.hibernate.persister.collection.CollectionPersister; import org.hibernate.type.CollectionType; -import org.hibernate.type.EntityType; /** - * Reassociates uninitialized proxies with the session + * A visitor able to reattach {@linkplain PersistentCollection collections} + * to the current session. + * * @author Gavin King */ public abstract class ProxyVisitor extends AbstractVisitor { @@ -21,15 +22,6 @@ public ProxyVisitor(EventSource session) { super(session); } - Object processEntity(Object value, EntityType entityType) { - if ( value != null ) { - getSession().getPersistenceContext().reassociateIfUninitializedProxy( value ); - // if it is an initialized proxy, let cascade - // handle it later on - } - return null; - } - /** * Has the owner of the collection changed since the collection * was snapshotted and detached? @@ -63,7 +55,7 @@ protected void reattachCollection(PersistentCollection collection, Collection } else { if ( !isCollectionSnapshotValid( collection ) ) { - throw new HibernateException( "could not re-associate uninitialized transient collection" ); + throw new HibernateException( "Could not reassociate uninitialized transient collection" ); } final var persister = metamodel.getCollectionDescriptor( collection.getRole() ); context.addUninitializedDetachedCollection( persister, collection ); diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/ReattachVisitor.java b/hibernate-core/src/main/java/org/hibernate/event/internal/ReattachVisitor.java index eb059e50b610..b4fc55cc6fde 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/ReattachVisitor.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/ReattachVisitor.java @@ -9,13 +9,15 @@ import org.hibernate.event.spi.EventSource; import org.hibernate.persister.collection.CollectionPersister; import org.hibernate.type.CompositeType; +import org.hibernate.type.EntityType; import org.hibernate.type.Type; import static org.hibernate.event.internal.EventListenerLogging.EVENT_LISTENER_LOGGER; import static org.hibernate.pretty.MessageHelper.collectionInfoString; /** - * Abstract superclass of visitors that reattach collections. + * Abstract superclass of visitors that reattach collections + * and reassociate uninitialized proxies with the session. * * @author Gavin King */ @@ -30,6 +32,17 @@ public ReattachVisitor(EventSource session, Object ownerIdentifier, Object owner this.owner = owner; } + @Override + Object processEntity(Object value, EntityType entityType) { + if ( value != null ) { + getSession().getPersistenceContext() + .reassociateIfUninitializedProxy( value ); + // if it is an initialized proxy, + // let cascade handle it later on + } + return null; + } + /** * Retrieve the identifier of the entity being visited. * diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/WrapVisitor.java b/hibernate-core/src/main/java/org/hibernate/event/internal/WrapVisitor.java index 90c6ec8f37d0..1c2d8ff95da5 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/WrapVisitor.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/WrapVisitor.java @@ -22,7 +22,7 @@ import static org.hibernate.persister.entity.AbstractEntityPersister.getCollectionKey; /** - * Wrap collections in a Hibernate collection wrapper. + * Wrap collections in {@linkplain PersistentCollection collection wrappers}. * * @author Gavin King */ diff --git a/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java b/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java index 1213a3bb9bb0..bed404713d0f 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java @@ -234,6 +234,10 @@ void missingArguments( @Message(value = "Unable to mark for rollback on TransientObjectException: ", id = 338) void unableToMarkForRollbackOnTransientObjectException(@Cause Exception e); + @LogMessage(level = ERROR) + @Message(value = "Unable to mark for rollback on DetachedObjectException: ", id = 339) + void unableToMarkForRollbackOnDetachedObjectException(@Cause Exception e); + @LogMessage(level = ERROR) @Message(value = "Could not release a cache lock: %s", id = 353) void unableToReleaseCacheLock(CacheException ce); diff --git a/hibernate-core/src/main/java/org/hibernate/internal/ExceptionConverterImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/ExceptionConverterImpl.java index 40fbce66c837..891014fef873 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/ExceptionConverterImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/ExceptionConverterImpl.java @@ -8,6 +8,7 @@ import java.sql.SQLException; import org.hibernate.AssertionFailure; +import org.hibernate.DetachedObjectException; import org.hibernate.HibernateException; import org.hibernate.JDBCException; import org.hibernate.LockOptions; @@ -141,6 +142,16 @@ else if ( exception instanceof TransientObjectException ) { //Spec 3.2.3 Synchronization rules return new IllegalStateException( exception ); } + else if ( exception instanceof DetachedObjectException ) { + try { + session.markForRollbackOnly(); + } + catch (Exception ne) { + //we do not want the subsequent exception to swallow the original one + CORE_LOGGER.unableToMarkForRollbackOnDetachedObjectException( ne ); + } + throw new IllegalArgumentException( exception ); + } else if ( exception instanceof TransactionSerializationException ) { final var converted = new RollbackException( exception.getMessage(), exception ); rollbackIfNecessary( converted ); 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 a37ec581e42c..ca26f946790a 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -624,7 +624,6 @@ public void lock(Object object, LockMode lockMode, LockOption... lockOptions) { private void fireLock(final LockEvent lockEvent) { checkOpen(); - checkEntityManaged( lockEvent.getEntityName(), lockEvent.getObject() ); try { pulseTransactionCoordinator(); checkTransactionNeededForLock( lockEvent.getLockMode() ); @@ -642,7 +641,11 @@ private void fireLock(final LockEvent lockEvent) { private void convertIfJpaBootstrap(RuntimeException exception, LockOptions lockOptions) { if ( !isJpaBootstrap() && exception instanceof HibernateException ) { - throw exception; + throw exception instanceof DetachedObjectException + // convert to IllegalArgumentException for backward compatibility + // TODO: drop this conversion in Hibernate 8 + ? new IllegalArgumentException( exception ) + : exception; } else if ( exception instanceof MappingException ) { // I believe this is now obsolete everywhere we do it, @@ -1308,7 +1311,6 @@ public void refresh(String entityName, Object object, RefreshContext refreshedAl private void fireRefresh(final RefreshEvent refreshEvent) { checkOpen(); - checkEntityManaged( refreshEvent.getEntityName(), refreshEvent.getObject() ); try { pulseTransactionCoordinator(); checkTransactionNeededForLock( refreshEvent.getLockMode() ); @@ -1327,7 +1329,6 @@ private void fireRefresh(final RefreshEvent refreshEvent) { private void fireRefresh(final RefreshContext refreshedAlready, final RefreshEvent refreshEvent) { // called from cascades checkOpenOrWaitingForAutoClose(); - checkEntityManaged( refreshEvent.getEntityName(), refreshEvent.getObject() ); try { pulseTransactionCoordinator(); eventListenerGroups.eventListenerGroup_REFRESH @@ -1339,12 +1340,6 @@ private void fireRefresh(final RefreshContext refreshedAlready, final RefreshEve } } - private void checkEntityManaged(String entityName, Object entity) { - if ( !isManaged( entity ) ) { - throw new IllegalArgumentException( "Given entity is not associated with the persistence context" ); - } - } - @Override public boolean isManaged(Object entity) { try { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/immutable/entitywithmutablecollection/AbstractEntityWithManyToManyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/immutable/entitywithmutablecollection/AbstractEntityWithManyToManyTest.java index 3f18085653dc..20689ea96ab3 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/immutable/entitywithmutablecollection/AbstractEntityWithManyToManyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/immutable/entitywithmutablecollection/AbstractEntityWithManyToManyTest.java @@ -6,7 +6,6 @@ import java.util.Iterator; -import org.hibernate.LockMode; import org.hibernate.StaleObjectStateException; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SessionImplementor; @@ -28,7 +27,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -167,10 +165,6 @@ public void testCreateWithNonEmptyManyToManyCollectionOfExisting(SessionFactoryS scope.inTransaction( s -> { - assertThrows(IllegalArgumentException.class, - () -> s.lock( c, LockMode.NONE ), - "Given entity is not associated with the persistence context" - ); Plan p = new Plan( "plan" ); p.addContract( s.get(Contract.class, c.getId()) ); s.persist( p ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/LockTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/LockTest.java index 84444c876bed..4bae66941aa6 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/LockTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/LockTest.java @@ -405,10 +405,9 @@ public void testUpdateWithPessimisticReadLockWithoutNoWait() { } protected String updateStatement() { - if( SQLServerDialect.class.isAssignableFrom( getDialect().getClass() ) ) { - return "UPDATE Lock_ WITH(NOWAIT) SET name = :name where id = :id"; - } - return "UPDATE Lock_ SET name = :name where id = :id"; + return SQLServerDialect.class.isAssignableFrom( getDialect().getClass() ) + ? "UPDATE Lock_ WITH(NOWAIT) SET name = :name where id = :id" + : "UPDATE Lock_ SET name = :name where id = :id"; } @Test diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ops/PersistTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ops/PersistTest.java index 43beb51ea771..4e45f0c10ce2 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ops/PersistTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ops/PersistTest.java @@ -6,6 +6,8 @@ import java.util.ArrayList; import java.util.Collection; + +import jakarta.persistence.EntityExistsException; import jakarta.persistence.PersistenceException; import jakarta.persistence.RollbackException; @@ -177,7 +179,7 @@ public void testCreateExceptionWithGeneratedId(EntityManagerFactoryScope scope) entityManager.persist( dupe ); fail(); } - catch (PersistenceException poe) { + catch (EntityExistsException eee) { //verify that an exception is thrown! } finally { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedEntityManagerArgumentsTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedEntityManagerArgumentsTest.java index c8f10d88b069..227859375bea 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedEntityManagerArgumentsTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedEntityManagerArgumentsTest.java @@ -32,7 +32,7 @@ class DetachedEntityManagerArgumentsTest { c.accept( thing ); } catch ( IllegalArgumentException e ) { - assertTrue( e.getMessage().startsWith( "Given entity is not associated with the persistence context" ) ); + assertTrue( e.getMessage().contains( "Given entity is not associated with the persistence context" ) ); } } ); } ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedSessionArgumentsTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedSessionArgumentsTest.java index 714bc666dca9..3dd77f8b88eb 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedSessionArgumentsTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedSessionArgumentsTest.java @@ -38,7 +38,7 @@ class DetachedSessionArgumentsTest { c.accept( thing ); } catch ( IllegalArgumentException e ) { - assertTrue( e.getMessage().startsWith( "Given entity is not associated with the persistence context" ) ); + assertTrue( e.getMessage().contains( "Given entity is not associated with the persistence context" ) ); } } ); } ); From 7a4843c34649f19eb84fc4377f44c92cae1ffe30 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 24 Nov 2025 11:42:14 +0100 Subject: [PATCH 2/2] cleanups after introduction of DetachedObjectException --- .../hibernate/engine/internal/ProxyUtil.java | 107 ++++++++++++++++++ .../internal/StatefulPersistenceContext.java | 73 ------------ .../engine/spi/PersistenceContext.java | 13 +-- .../internal/DefaultLockEventListener.java | 9 +- .../internal/DefaultMergeEventListener.java | 3 +- .../internal/DefaultPersistEventListener.java | 15 ++- .../internal/DefaultRefreshEventListener.java | 65 +++++------ 7 files changed, 156 insertions(+), 129 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/engine/internal/ProxyUtil.java diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/ProxyUtil.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/ProxyUtil.java new file mode 100644 index 000000000000..8704e10a3c87 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/ProxyUtil.java @@ -0,0 +1,107 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.engine.internal; + +import org.hibernate.DetachedObjectException; +import org.hibernate.PersistentObjectException; +import org.hibernate.bytecode.enhance.spi.interceptor.BytecodeLazyAttributeInterceptor; +import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementAsProxyLazinessInterceptor; +import org.hibernate.engine.spi.SharedSessionContractImplementor; + +import static org.hibernate.engine.internal.ManagedTypeHelper.asPersistentAttributeInterceptable; +import static org.hibernate.engine.internal.ManagedTypeHelper.isPersistentAttributeInterceptable; +import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer; + +/** + * @author Gavin King + * @since 7.2 + */ +public class ProxyUtil { + + /** + * Get the entity instance underlying the given proxy, throwing + * an exception if the proxy is uninitialized. If the given + * object is not a proxy, simply return the argument. + */ + public static Object assertInitialized(Object maybeProxy) { + final var lazyInitializer = extractLazyInitializer( maybeProxy ); + if ( lazyInitializer != null ) { + if ( lazyInitializer.isUninitialized() ) { + throw new PersistentObjectException( "Object was an uninitialized proxy for " + + lazyInitializer.getEntityName() ); + } + //unwrap the object and return + return lazyInitializer.getImplementation(); + } + else { + return maybeProxy; + } + } + + /** + * Get the entity instance underlying the given proxy, forcing + * initialization if the proxy is uninitialized. If the given + * object is not a proxy, simply return the argument. + * @throws DetachedObjectException if the given proxy does not + * belong to the given session + */ + public static Object forceInitialize(Object maybeProxy, SharedSessionContractImplementor session) { + final var lazyInitializer = extractLazyInitializer( maybeProxy ); + if ( lazyInitializer != null ) { + if ( lazyInitializer.getSession() != session ) { + throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); + } + //initialize + unwrap the object and return it + return lazyInitializer.getImplementation(); + } + else if ( isPersistentAttributeInterceptable( maybeProxy ) ) { + final var interceptor = + asPersistentAttributeInterceptable( maybeProxy ) + .$$_hibernate_getInterceptor(); + if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor lazinessInterceptor ) { + if ( lazinessInterceptor.getLinkedSession() != session ) { + throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); + } + lazinessInterceptor.forceInitialize( maybeProxy, null ); + } + return maybeProxy; + } + else { + return maybeProxy; + } + } + + /** + * Determine of the given proxy is uninitialized. If the given + * object is not a proxy, simply return false. + * @throws DetachedObjectException if the given proxy does not + * belong to the given session + */ + public static boolean isUninitialized(Object value, SharedSessionContractImplementor session) { + // could be a proxy + final var lazyInitializer = extractLazyInitializer( value ); + if ( lazyInitializer != null ) { + if ( lazyInitializer.getSession() != session ) { + throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); + } + return lazyInitializer.isUninitialized(); + } + // or an uninitialized enhanced entity ("bytecode proxy") + else if ( isPersistentAttributeInterceptable( value ) ) { + final var interceptor = + (BytecodeLazyAttributeInterceptor) + asPersistentAttributeInterceptable( value ) + .$$_hibernate_getInterceptor(); + if ( interceptor != null && interceptor.getLinkedSession() != session ) { + throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); + } + return interceptor instanceof EnhancementAsProxyLazinessInterceptor enhancementInterceptor + && !enhancementInterceptor.isInitialized(); + } + else { + return false; + } + } +} 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 c4335d0a76e7..b8ca5d667396 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 @@ -23,13 +23,11 @@ import java.util.function.Supplier; import org.hibernate.AssertionFailure; -import org.hibernate.DetachedObjectException; import org.hibernate.Hibernate; import org.hibernate.HibernateException; import org.hibernate.LockMode; import org.hibernate.MappingException; import org.hibernate.NonUniqueObjectException; -import org.hibernate.PersistentObjectException; import org.hibernate.bytecode.enhance.spi.interceptor.BytecodeLazyAttributeInterceptor; import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementAsProxyLazinessInterceptor; import org.hibernate.collection.spi.PersistentCollection; @@ -723,33 +721,6 @@ else if ( isPersistentAttributeInterceptable( value ) ) { } } - @Override - public boolean isUninitializedProxy(Object value) throws MappingException { - // could be a proxy - final var lazyInitializer = extractLazyInitializer( value ); - if ( lazyInitializer != null ) { - if ( lazyInitializer.getSession() != session ) { - throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); - } - return lazyInitializer.isUninitialized(); - } - // or an uninitialized enhanced entity ("bytecode proxy") - else if ( isPersistentAttributeInterceptable( value ) ) { - final var interceptor = - (BytecodeLazyAttributeInterceptor) - asPersistentAttributeInterceptable( value ) - .$$_hibernate_getInterceptor(); - if ( interceptor != null && interceptor.getLinkedSession() != session ) { - throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); - } - return interceptor instanceof EnhancementAsProxyLazinessInterceptor enhancementInterceptor - && !enhancementInterceptor.isInitialized(); - } - else { - return false; - } - } - @Override public void reassociateProxy(Object value, Object id) throws MappingException { final var lazyInitializer = extractLazyInitializer( value ); @@ -787,50 +758,6 @@ private void reassociateProxy(LazyInitializer li, HibernateProxy proxy) { } } - @Override - public Object unproxy(Object maybeProxy) throws HibernateException { - final var lazyInitializer = extractLazyInitializer( maybeProxy ); - if ( lazyInitializer != null ) { - if ( lazyInitializer.isUninitialized() ) { - throw new PersistentObjectException( "Object was an uninitialized proxy for " - + lazyInitializer.getEntityName() ); - } - //unwrap the object and return - return lazyInitializer.getImplementation(); - } - else { - return maybeProxy; - } - } - - @Override - public Object unproxyLoadingIfNecessary(final Object maybeProxy) throws HibernateException { - final var lazyInitializer = extractLazyInitializer( maybeProxy ); - if ( lazyInitializer != null ) { - if ( lazyInitializer.getSession() != session ) { - throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); - } - //initialize + unwrap the object and return it - return lazyInitializer.getImplementation(); - } - else if ( isPersistentAttributeInterceptable( maybeProxy ) ) { - final var interceptor = - asPersistentAttributeInterceptable( maybeProxy ) - .$$_hibernate_getInterceptor(); - if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor lazinessInterceptor ) { - if ( lazinessInterceptor.getLinkedSession() != session ) { - throw new DetachedObjectException( "Given proxy does not belong to this persistence context" ); - } - lazinessInterceptor.forceInitialize( maybeProxy, null ); - } - return maybeProxy; - } - else { - return maybeProxy; - } - } - - @Override public Object unproxyAndReassociate(final Object maybeProxy) throws HibernateException { final var lazyInitializer = extractLazyInitializer( maybeProxy ); 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 0f5f509018e2..212d16f6679e 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 @@ -16,6 +16,7 @@ import org.hibernate.Internal; import org.hibernate.LockMode; import org.hibernate.collection.spi.PersistentCollection; +import org.hibernate.engine.internal.ProxyUtil; import org.hibernate.internal.util.MarkerObject; import org.hibernate.persister.collection.CollectionPersister; import org.hibernate.persister.entity.EntityPersister; @@ -293,14 +294,12 @@ EntityEntry addReferenceEntry( * Get the entity instance underlying the given proxy, throwing * an exception if the proxy is uninitialized. If the given object * is not a proxy, simply return the argument. + * @deprecated No longer used */ - Object unproxy(Object maybeProxy); - - @Incubating - Object unproxyLoadingIfNecessary(final Object maybeProxy); - - @Incubating - boolean isUninitializedProxy(Object value); + @Deprecated(since = "7.2", forRemoval = true) + default Object unproxy(Object maybeProxy) { + return ProxyUtil.assertInitialized( maybeProxy ); + } /** * Possibly unproxy the given reference and reassociate it with the current session. diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java index 20f99c16e615..e37b4ff938f1 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java @@ -11,7 +11,7 @@ import org.hibernate.event.spi.LockEventListener; - +import static org.hibernate.engine.internal.ProxyUtil.forceInitialize; import static org.hibernate.loader.ast.internal.LoaderHelper.upgradeLock; /** @@ -41,16 +41,15 @@ public void onLock(LockEvent event) throws HibernateException { } final var source = event.getSession(); - final var persistenceContext = source.getPersistenceContextInternal(); - final Object entity = persistenceContext.unproxyLoadingIfNecessary( instance ); + final Object entity = forceInitialize( instance, source ); //TODO: if instance was an uninitialized proxy, this is inefficient, // resulting in two SQL selects - final var entry = persistenceContext.getEntry( entity ); + final var entry = source.getPersistenceContextInternal().getEntry( entity ); if ( entry == null && instance == entity ) { throw new DetachedObjectException( "Given entity is not associated with the persistence context" ); } - upgradeLock( entity, entry, event.getLockOptions(), event.getSession() ); + upgradeLock( entity, entry, event.getLockOptions(), source ); } } 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 6e89f91e6353..191f2f5f91ac 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 @@ -40,6 +40,7 @@ import static org.hibernate.engine.internal.ManagedTypeHelper.isHibernateProxy; import static org.hibernate.engine.internal.ManagedTypeHelper.isPersistentAttributeInterceptable; import static org.hibernate.engine.internal.ManagedTypeHelper.isSelfDirtinessTracker; +import static org.hibernate.engine.internal.ProxyUtil.assertInitialized; import static org.hibernate.event.internal.EntityState.getEntityState; import static org.hibernate.event.internal.EventListenerLogging.EVENT_LISTENER_LOGGER; import static org.hibernate.event.internal.EventUtil.getLoggableName; @@ -515,7 +516,7 @@ private static Object unproxyManagedForDetachedMerging( EntityPersister persister, EventSource source) { if ( isHibernateProxy( managed ) ) { - return source.getPersistenceContextInternal().unproxy( managed ); + return assertInitialized( managed ); } if ( isPersistentAttributeInterceptable( incoming ) diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPersistEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPersistEventListener.java index 2c044d8f058a..e942e6842dff 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPersistEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPersistEventListener.java @@ -18,6 +18,7 @@ import org.hibernate.jpa.event.spi.CallbackRegistryConsumer; import org.hibernate.persister.entity.EntityPersister; +import static org.hibernate.engine.internal.ProxyUtil.assertInitialized; import static org.hibernate.event.internal.EntityState.getEntityState; import static org.hibernate.event.internal.EventListenerLogging.EVENT_LISTENER_LOGGER; import static org.hibernate.event.internal.EventUtil.getLoggableName; @@ -118,14 +119,13 @@ protected void entityIsPersistent(PersistEvent event, PersistContext createCache final var source = event.getSession(); final String entityName = event.getEntityName(); //TODO: check that entry.getIdentifier().equals(requestedId) - final Object entity = source.getPersistenceContextInternal().unproxy( event.getObject() ); + final Object entity = assertInitialized( event.getObject() ); + final var persister = source.getEntityPersister( entityName, entity ); if ( EVENT_LISTENER_LOGGER.isTraceEnabled() ) { - final var persister = source.getEntityPersister( entityName, entity ); EVENT_LISTENER_LOGGER.ignoringPersistentInstance( infoString( entityName, persister.getIdentifier( entity ) ) ); } if ( createCache.add( entity ) ) { - final var persister = source.getEntityPersister( entityName, entity ); justCascade( createCache, source, entity, persister ); } } @@ -138,21 +138,20 @@ private void justCascade(PersistContext createCache, EventSource source, Object protected void entityIsTransient(PersistEvent event, PersistContext createCache) { EVENT_LISTENER_LOGGER.persistingTransientInstance(); - final var source = event.getSession(); - final Object entity = source.getPersistenceContextInternal().unproxy( event.getObject() ); + final Object entity = assertInitialized( event.getObject() ); if ( createCache.add( entity ) ) { + final var source = event.getSession(); saveWithGeneratedId( entity, event.getEntityName(), createCache, source, false ); } } private void entityIsDeleted(PersistEvent event, PersistContext createCache) { final var source = event.getSession(); - final Object entity = source.getPersistenceContextInternal().unproxy( event.getObject() ); + final Object entity = assertInitialized( event.getObject() ); final var persister = source.getEntityPersister( event.getEntityName(), entity ); if ( EVENT_LISTENER_LOGGER.isTraceEnabled() ) { - final Object id = persister.getIdentifier( entity, source ); EVENT_LISTENER_LOGGER.unschedulingEntityDeletion( - infoString( persister, id, source.getFactory() ) ); + infoString( persister, persister.getIdentifier( entity, source ), source.getFactory() ) ); } if ( createCache.add( entity ) ) { justCascade( createCache, source, entity, persister ); diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java index 0fde36a8b0ad..369dc220c61c 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java @@ -26,6 +26,8 @@ import org.hibernate.type.ComponentType; import org.hibernate.type.Type; +import static org.hibernate.engine.internal.ProxyUtil.forceInitialize; +import static org.hibernate.engine.internal.ProxyUtil.isUninitialized; import static org.hibernate.event.internal.EventListenerLogging.EVENT_LISTENER_LOGGER; import static org.hibernate.pretty.MessageHelper.infoString; import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer; @@ -50,32 +52,30 @@ public void onRefresh(RefreshEvent event) throws HibernateException { */ @Override public void onRefresh(RefreshEvent event, RefreshContext refreshedAlready) { - final var source = event.getSession(); - final var persistenceContext = source.getPersistenceContextInternal(); final Object object = event.getObject(); if ( object == null ) { throw new NullPointerException( "Attempted to refresh null" ); } - else if ( persistenceContext.isUninitializedProxy( object ) ) { - handleUninitializedProxy( event, refreshedAlready, source, object, persistenceContext ); - } else { - final Object entity = persistenceContext.unproxyLoadingIfNecessary( object ); - if ( refreshedAlready.add( entity) ) { - refresh( event, refreshedAlready, entity ); + final var source = event.getEventSource(); + if ( isUninitialized( object, source ) ) { + handleUninitializedProxy( event, refreshedAlready ); } else { - EVENT_LISTENER_LOGGER.alreadyRefreshed(); + final Object entity = forceInitialize( object, source ); + if ( refreshedAlready.add( entity ) ) { + refresh( event, refreshedAlready, entity ); + } + else { + EVENT_LISTENER_LOGGER.alreadyRefreshed(); + } } } } - private static void handleUninitializedProxy( - RefreshEvent event, - RefreshContext refreshedAlready, - EventSource source, - Object object, - PersistenceContext persistenceContext) { + private static void handleUninitializedProxy(RefreshEvent event, RefreshContext refreshedAlready) { + final var source = event.getEventSource(); + final Object object = event.getObject(); final boolean isTransient = !source.isManaged( object ); // If refreshAlready is nonempty then the refresh is the result of a cascade refresh and the // refresh of the parent will take care of initializing the lazy entity and setting the @@ -90,8 +90,8 @@ private static void handleUninitializedProxy( persister, lazyInitializer, null, - persister.getIdentifier( object, event.getSession() ), - persistenceContext + persister.getIdentifier( object, source ), + source.getPersistenceContextInternal() ); if ( lazyInitializer != null ) { refreshedAlready.add( lazyInitializer.getImplementation() ); @@ -124,21 +124,18 @@ private static void refresh(RefreshEvent event, RefreshContext refreshedAlready, final var persistenceContext = source.getPersistenceContextInternal(); final var entry = persistenceContext.getEntry( object ); - final EntityPersister persister; - final Object id; if ( entry == null ) { throw new DetachedObjectException( "Given entity is not associated with the persistence context" ); } - else { - persister = entry.getPersister(); - id = entry.getId(); - if ( EVENT_LISTENER_LOGGER.isTraceEnabled() ) { - EVENT_LISTENER_LOGGER.refreshing( - infoString( persister, id, event.getFactory() ) ); - } - if ( !entry.isExistsInDatabase() ) { - throw new UnresolvableObjectException( id, persister.getEntityName() ); - } + + final EntityPersister persister = entry.getPersister(); + final Object id = entry.getId(); + if ( EVENT_LISTENER_LOGGER.isTraceEnabled() ) { + EVENT_LISTENER_LOGGER.refreshing( + infoString( persister, id, event.getFactory() ) ); + } + if ( !entry.isExistsInDatabase() ) { + throw new UnresolvableObjectException( id, persister.getEntityName() ); } // cascade the refresh prior to refreshing this entity @@ -151,13 +148,11 @@ private static void refresh(RefreshEvent event, RefreshContext refreshedAlready, refreshedAlready ); - if ( entry != null ) { - persistenceContext.removeEntityHolder( entry.getEntityKey() ); - if ( persister.hasCollections() ) { - new EvictVisitor( source, object ).process( object, persister ); - } - persistenceContext.removeEntry( object ); + persistenceContext.removeEntityHolder( entry.getEntityKey() ); + if ( persister.hasCollections() ) { + new EvictVisitor( source, object ).process( object, persister ); } + persistenceContext.removeEntry( object ); evictEntity( object, persister, id, source ); evictCachedCollections( persister, id, source );