HHH-18169 disallow refresh/lock for detached instance

Signed-off-by: Gavin King <[email protected]>

Author: gavinking

documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc

@@ -933,44 +933,6 @@ And finally, serialization will make the deserialized form be detached (the orig
 
 Detached data can still be manipulated, however, the persistence context will no longer automatically know about these modifications, and the application will need to intervene to make the changes persistent again.
 
-[[pc-detach-reattach]]
-==== Reattaching detached data
-
-Reattachment is the process of taking an incoming entity instance that is in the detached state and re-associating it with the current persistence context.
-
-[IMPORTANT]
-====
-Jakarta Persistence does not support reattaching detached data. This is only available through Hibernate `org.hibernate.Session`.
-====
-
-[[pc-detach-reattach-lock-example]]
-.Reattaching a detached entity using `lock`
-====
-[source, java, indent=0]
-----
-include::{example-dir-pc}/PersistenceContextTest.java[tags=pc-detach-reattach-lock-example]
-----
-====
-
-[[pc-detach-reattach-saveOrUpdate-example]]
-.Reattaching a detached entity using `saveOrUpdate`
-====
-[source, java, indent=0]
-----
-include::{example-dir-pc}/PersistenceContextTest.java[tags=pc-detach-reattach-saveOrUpdate-example]
-----
-====
-
-[NOTE]
-====
-The method name `update` is a bit misleading here.
-It does not mean that an `SQL` `UPDATE` is immediately performed.
-It does, however, mean that an `SQL` `UPDATE` will be performed when the persistence context is flushed since Hibernate does not know its previous state against which to compare for changes.
-If the entity is mapped with `select-before-update`, Hibernate will pull the current state from the database and see if an update is needed.
-====
-
-Provided the entity is detached, `update` and `saveOrUpdate` operate exactly the same.
-
 [[pc-merge]]
 ==== Merging detached data
 
@@ -1274,17 +1236,6 @@ include::{example-dir-pc}/CascadeDetachTest.java[tags=pc-cascade-detach-example]
 Although unintuitively, `CascadeType.LOCK` does not propagate a lock request from a parent entity to its children.
 Such a use case requires the use of the `PessimisticLockScope.EXTENDED` value of the `jakarta.persistence.lock.scope` property.
 
-However, `CascadeType.LOCK` allows us to reattach a parent entity along with its children to the currently running Persistence Context.
-
-[[pc-cascade-lock-example]]
-.`CascadeType.LOCK` example
-====
-[source, java, indent=0]
-----
-include::{example-dir-pc}/CascadeLockTest.java[tags=pc-cascade-lock-example]
-----
-====
-
 [[pc-cascade-refresh]]
 ==== `CascadeType.REFRESH`
 

hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java

@@ -337,7 +337,7 @@ public SessionFactoryOptionsBuilder(StandardServiceRegistry serviceRegistry, Boo
 		this.allowRefreshDetachedEntity = configurationService.getSetting(
 				ALLOW_REFRESH_DETACHED_ENTITY,
 				BOOLEAN,
-				true
+				false
 		);
 
 		this.flushBeforeCompletionEnabled = configurationService.getSetting( FLUSH_BEFORE_COMPLETION, BOOLEAN, true );

hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java

@@ -659,6 +659,7 @@ private void fireLock(Object object, LockOptions options) {
 
 	private void fireLock(LockEvent event) {
 		checkOpen();
+		checkEntityManaged( event.getEntityName(), event.getObject() );
 		pulseTransactionCoordinator();
 		fastSessionServices.eventListenerGroup_LOCK
 				.fireEventOnEachListener( event, LockEventListener::onLock );
@@ -1163,18 +1164,7 @@ public void refresh(String entityName, Object object, RefreshContext refreshedAl
 
 	private void fireRefresh(final RefreshEvent event) {
 		try {
-			if ( !getSessionFactory().getSessionFactoryOptions().isAllowRefreshDetachedEntity() ) {
-				if ( event.getEntityName() != null ) {
-					if ( !contains( event.getEntityName(), event.getObject() ) ) {
-						throw new IllegalArgumentException( "Entity not managed" );
-					}
-				}
-				else {
-					if ( !contains( event.getObject() ) ) {
-						throw new IllegalArgumentException( "Entity not managed" );
-					}
-				}
-			}
+			checkEntityManaged( event.getEntityName(), event.getObject() );
 			pulseTransactionCoordinator();
 			fastSessionServices.eventListenerGroup_REFRESH
 					.fireEventOnEachListener( event, RefreshEventListener::onRefresh );
@@ -1195,6 +1185,7 @@ private void fireRefresh(final RefreshEvent event) {
 
 	private void fireRefresh(final RefreshContext refreshedAlready, final RefreshEvent event) {
 		try {
+			checkEntityManaged( event.getEntityName(), event.getObject() );
 			pulseTransactionCoordinator();
 			fastSessionServices.eventListenerGroup_REFRESH
 					.fireEventOnEachListener( event, refreshedAlready, RefreshEventListener::onRefresh );
@@ -1207,6 +1198,18 @@ private void fireRefresh(final RefreshContext refreshedAlready, final RefreshEve
 		}
 	}
 
+	private void checkEntityManaged(String entityName, Object entity) {
+		if ( !getSessionFactory().getSessionFactoryOptions().isAllowRefreshDetachedEntity() ) {
+			if ( !managed( entityName, entity ) ) {
+				throw new IllegalArgumentException(
+						"Given entity is not associated with the persistence context" );
+			}
+		}
+	}
+
+	private boolean managed(String entityName, Object entity) {
+		return entityName == null ? contains( entity ) : contains( entityName, entity );
+	}
 
 	// replicate() operations ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java

@@ -86,7 +86,6 @@
 import static org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer.UNFETCHED_PROPERTY;
 import static org.hibernate.engine.internal.ManagedTypeHelper.asPersistentAttributeInterceptable;
 import static org.hibernate.engine.internal.ManagedTypeHelper.isPersistentAttributeInterceptable;
-import static org.hibernate.internal.log.LoggingHelper.toLoggableString;
 import static org.hibernate.metamodel.mapping.ForeignKeyDescriptor.Nature.TARGET;
 import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer;
 
@@ -163,12 +162,9 @@ public EntityInitializerData(EntityInitializerImpl initializer, RowProcessingSta
 			lockMode = rowProcessingState.determineEffectiveLockMode( initializer.sourceAlias );
 			if ( initializer.isResultInitializer() ) {
 				uniqueKeyAttributePath = rowProcessingState.getEntityUniqueKeyAttributePath();
-				if ( uniqueKeyAttributePath != null ) {
-					uniqueKeyPropertyTypes = initializer.getParentEntityAttributeTypes( uniqueKeyAttributePath );
-				}
-				else {
-					uniqueKeyPropertyTypes = null;
-				}
+				uniqueKeyPropertyTypes = uniqueKeyAttributePath != null
+						? initializer.getParentEntityAttributeTypes( uniqueKeyAttributePath )
+						: null;
 				canUseEmbeddedIdentifierInstanceAsEntity = rowProcessingState.getEntityId() != null
 						&& initializer.couldUseEmbeddedIdentifierInstanceAsEntity;
 			}
@@ -178,13 +174,10 @@ public EntityInitializerData(EntityInitializerImpl initializer, RowProcessingSta
 				canUseEmbeddedIdentifierInstanceAsEntity = false;
 			}
 			hasCallbackActions = rowProcessingState.hasCallbackActions();
-			if ( initializer.discriminatorAssembler == null
-					|| rowProcessingState.isQueryCacheHit() && entityDescriptor.useShallowQueryCacheLayout() && !entityDescriptor.storeDiscriminatorInShallowQueryCacheLayout() ) {
-				defaultConcreteDescriptor = entityDescriptor;
-			}
-			else {
-				defaultConcreteDescriptor = null;
-			}
+			defaultConcreteDescriptor =
+					hasConcreteDescriptor( rowProcessingState, initializer.discriminatorAssembler, entityDescriptor )
+							? entityDescriptor
+							: null;
 		}
 
 		/*
@@ -206,6 +199,16 @@ public EntityInitializerData(EntityInitializerData original) {
 		}
 	}
 
+	private static boolean hasConcreteDescriptor(
+			RowProcessingState rowProcessingState,
+			BasicResultAssembler<?> discriminatorAssembler,
+			EntityPersister entityDescriptor) {
+		return discriminatorAssembler == null
+			|| rowProcessingState.isQueryCacheHit()
+				&& entityDescriptor.useShallowQueryCacheLayout()
+				&& !entityDescriptor.storeDiscriminatorInShallowQueryCacheLayout();
+	}
+
 	public EntityInitializerImpl(
 			EntityResultGraphNode resultDescriptor,
 			String sourceAlias,
@@ -229,8 +232,9 @@ public EntityInitializerImpl(
 				: entityDescriptor.getRootEntityDescriptor().getEntityPersister();
 		keyTypeForEqualsHashCode = entityDescriptor.getIdentifierType().getTypeForEqualsHashCode();
 		// The id can only be the entity instance if this is a non-aggregated id that has no containing class
-		couldUseEmbeddedIdentifierInstanceAsEntity = entityDescriptor.getIdentifierMapping() instanceof CompositeIdentifierMapping
-				&& !( (CompositeIdentifierMapping) entityDescriptor.getIdentifierMapping() ).hasContainingClass();
+		couldUseEmbeddedIdentifierInstanceAsEntity =
+				entityDescriptor.getIdentifierMapping() instanceof CompositeIdentifierMapping
+						&& !( (CompositeIdentifierMapping) entityDescriptor.getIdentifierMapping() ).hasContainingClass();
 
 		this.navigablePath = resultDescriptor.getNavigablePath();
 		this.sourceAlias = sourceAlias;
@@ -258,7 +262,8 @@ public EntityInitializerImpl(
 			hasKeyManyToOne = initializer != null && initializer.isLazyCapable();
 		}
 
-		assert entityDescriptor.hasSubclasses() == (discriminatorFetch != null) : "Discriminator should only be fetched if the entity has subclasses";
+		assert entityDescriptor.hasSubclasses() == (discriminatorFetch != null)
+				: "Discriminator should only be fetched if the entity has subclasses";
 		discriminatorAssembler = discriminatorFetch != null
 				? (BasicResultAssembler<?>) discriminatorFetch.createAssembler( this, creationState )
 				: null;
@@ -286,12 +291,16 @@ public EntityInitializerImpl(
 		final BitSet[] lazySets = new BitSet[subMappingTypes.size() + 1];
 		final BitSet[] maybeLazySets = new BitSet[subMappingTypes.size() + 1];
 		final MutabilityPlan[][] updatableAttributeMutabilityPlans = new MutabilityPlan[subMappingTypes.size() + 1][];
-		assemblers[rootEntityDescriptor.getSubclassId()] = new DomainResultAssembler[rootEntityDescriptor.getNumberOfFetchables()];
-		updatableAttributeMutabilityPlans[rootEntityDescriptor.getSubclassId()] = new MutabilityPlan[rootEntityDescriptor.getNumberOfAttributeMappings()];
+		assemblers[rootEntityDescriptor.getSubclassId()] =
+				new DomainResultAssembler[rootEntityDescriptor.getNumberOfFetchables()];
+		updatableAttributeMutabilityPlans[rootEntityDescriptor.getSubclassId()] =
+				new MutabilityPlan[rootEntityDescriptor.getNumberOfAttributeMappings()];
 
 		for ( EntityMappingType subMappingType : subMappingTypes ) {
-			assemblers[subMappingType.getSubclassId()] = new DomainResultAssembler[subMappingType.getNumberOfFetchables()];
-			updatableAttributeMutabilityPlans[subMappingType.getSubclassId()] = new MutabilityPlan[subMappingType.getNumberOfAttributeMappings()];
+			assemblers[subMappingType.getSubclassId()] =
+					new DomainResultAssembler[subMappingType.getNumberOfFetchables()];
+			updatableAttributeMutabilityPlans[subMappingType.getSubclassId()] =
+					new MutabilityPlan[subMappingType.getNumberOfAttributeMappings()];
 		}
 
 		boolean hasLazySubInitializers = false;
@@ -345,7 +354,8 @@ public EntityInitializerImpl(
 			}
 			for ( EntityMappingType subMappingType : declaringType.getSubMappingTypes() ) {
 				assemblers[subMappingType.getSubclassId()][stateArrayPosition] = stateAssembler;
-				updatableAttributeMutabilityPlans[subMappingType.getSubclassId()][stateArrayPosition] = updatableAttributeMutabilityPlans[subclassId][stateArrayPosition];
+				updatableAttributeMutabilityPlans[subMappingType.getSubclassId()][stateArrayPosition] =
+						updatableAttributeMutabilityPlans[subclassId][stateArrayPosition];
 				if ( subInitializer != null ) {
 					if ( subInitializers[subMappingType.getSubclassId()] == null ) {
 						subInitializers[subMappingType.getSubclassId()] = new Initializer<?>[size];
@@ -355,12 +365,14 @@ public EntityInitializerImpl(
 						maybeLazySets[subMappingType.getSubclassId()] = new BitSet(size);
 					}
 					subInitializers[subMappingType.getSubclassId()][stateArrayPosition] = subInitializer;
-					eagerSubInitializers[subMappingType.getSubclassId()][stateArrayPosition] = eagerSubInitializers[subclassId][stateArrayPosition];
-					collectionContainingSubInitializers[subMappingType.getSubclassId()][stateArrayPosition] = collectionContainingSubInitializers[subclassId][stateArrayPosition];
-					if (lazySets[subclassId].get( stateArrayPosition ) ) {
+					eagerSubInitializers[subMappingType.getSubclassId()][stateArrayPosition] =
+							eagerSubInitializers[subclassId][stateArrayPosition];
+					collectionContainingSubInitializers[subMappingType.getSubclassId()][stateArrayPosition] =
+							collectionContainingSubInitializers[subclassId][stateArrayPosition];
+					if ( lazySets[subclassId].get( stateArrayPosition ) ) {
 						lazySets[subMappingType.getSubclassId()].set( stateArrayPosition );
 					}
-					if (maybeLazySets[subclassId].get( stateArrayPosition ) ) {
+					if ( maybeLazySets[subclassId].get( stateArrayPosition ) ) {
 						maybeLazySets[subMappingType.getSubclassId()].set( stateArrayPosition );
 					}
 				}
@@ -411,9 +423,10 @@ public EntityInitializerImpl(
 
 		this.assemblers = assemblers;
 		this.subInitializers = subInitializers;
-		this.subInitializersForResolveFromInitialized = rootEntityDescriptor.getBytecodeEnhancementMetadata().isEnhancedForLazyLoading()
-				? subInitializers
-				: eagerSubInitializers;
+		this.subInitializersForResolveFromInitialized =
+				rootEntityDescriptor.getBytecodeEnhancementMetadata().isEnhancedForLazyLoading()
+						? subInitializers
+						: eagerSubInitializers;
 		this.collectionContainingSubInitializers = collectionContainingSubInitializers;
 		this.lazySets = Arrays.stream( lazySets ).map( ImmutableBitSet::valueOf ).toArray( ImmutableBitSet[]::new );
 		this.maybeLazySets = Arrays.stream( maybeLazySets )
@@ -531,7 +544,8 @@ protected void resolveKey(EntityInitializerData data, boolean entityKeyOnly) {
 		}
 		else {
 			//noinspection unchecked
-			final Initializer<InitializerData> initializer = (Initializer<InitializerData>) identifierAssembler.getInitializer();
+			final Initializer<InitializerData> initializer =
+					(Initializer<InitializerData>) identifierAssembler.getInitializer();
 			if ( initializer != null ) {
 				final InitializerData subData = initializer.getData( rowProcessingState );
 				initializer.resolveKey( subData );
@@ -785,7 +799,8 @@ public boolean isResultInitializer() {
 	}
 
 	private void deepCopy(EntityPersister containerDescriptor, Object[] source, Object[] target) {
-		final MutabilityPlan<Object>[] updatableAttributeMutabilityPlan = updatableAttributeMutabilityPlans[containerDescriptor.getSubclassId()];
+		final MutabilityPlan<Object>[] updatableAttributeMutabilityPlan =
+				updatableAttributeMutabilityPlans[containerDescriptor.getSubclassId()];
 		for ( int i = 0; i < updatableAttributeMutabilityPlan.length; i++ ) {
 			final Object sourceValue = source[i];
 			if ( updatableAttributeMutabilityPlan[i] != null
@@ -830,11 +845,7 @@ public Object getTargetInstance(EntityInitializerData data) {
 	protected Type[] getParentEntityAttributeTypes(String attributeName) {
 		Type[] types = parentEntityAttributeTypes.get( attributeName );
 		if ( types == null ) {
-			types = new Type[
-					entityDescriptor.getRootEntityDescriptor()
-							.getSubclassEntityNames()
-							.size()
-					];
+			types = new Type[entityDescriptor.getRootEntityDescriptor().getSubclassEntityNames().size()];
 			initializeAttributeType( types, entityDescriptor, attributeName );
 			for ( EntityMappingType subMappingType : entityDescriptor.getSubMappingTypes() ) {
 				initializeAttributeType( types, subMappingType.getEntityPersister(), attributeName );
@@ -855,12 +866,12 @@ protected void initializeAttributeType(Type[] attributeTypes, EntityPersister en
 			@Nullable BasicResultAssembler<?> discriminatorAssembler,
 			EntityPersister entityDescriptor)
 			throws WrongClassException {
-		if ( discriminatorAssembler == null
-				|| rowProcessingState.isQueryCacheHit() && entityDescriptor.useShallowQueryCacheLayout() && !entityDescriptor.storeDiscriminatorInShallowQueryCacheLayout() ) {
+		if ( hasConcreteDescriptor( rowProcessingState, discriminatorAssembler, entityDescriptor ) ) {
 			return entityDescriptor;
 		}
 		else {
-			assert entityDescriptor.hasSubclasses() : "Reading a discriminator from a result set should only happen if the entity has subclasses";
+			assert entityDescriptor.hasSubclasses()
+					: "Reading a discriminator from a result set should only happen if the entity has subclasses";
 			final EntityDiscriminatorMapping discriminatorMapping = entityDescriptor.getDiscriminatorMapping();
 			assert discriminatorMapping != null;
 			final Object discriminator = discriminatorAssembler.extractRawValue( rowProcessingState );
@@ -888,9 +899,16 @@ protected void initializeAttributeType(Type[] attributeTypes, EntityPersister en
 	}
 
 	protected boolean useEmbeddedIdentifierInstanceAsEntity(EntityInitializerData data) {
-		return data.canUseEmbeddedIdentifierInstanceAsEntity
-				&& ( data.concreteDescriptor = determineConcreteEntityDescriptor( data.getRowProcessingState(), discriminatorAssembler, entityDescriptor ) ) != null
+		if ( data.canUseEmbeddedIdentifierInstanceAsEntity ) {
+			data.concreteDescriptor =
+					determineConcreteEntityDescriptor( data.getRowProcessingState(),
+							discriminatorAssembler, entityDescriptor );
+			return data.concreteDescriptor != null
 				&& data.concreteDescriptor.isInstance( data.getRowProcessingState().getEntityId() );
+		}
+		else {
+			return false;
+		}
 	}
 
 	@Override
@@ -918,7 +936,7 @@ public void resolveInstance(Object instance, EntityInitializerData data) {
 				resolveKey( data );
 				assert data.getState() == State.MISSING;
 				assert referencedModelPart instanceof ToOneAttributeMapping
-						&& ( (ToOneAttributeMapping) referencedModelPart ).getSideNature() == TARGET;
+					&& ( (ToOneAttributeMapping) referencedModelPart ).getSideNature() == TARGET;
 				return;
 			}
 			// If the entity initializer is null, we know the entity is fully initialized,
@@ -1132,7 +1150,7 @@ protected void upgradeLockMode(EntityInitializerData data) {
 
 	private boolean isProxyInstance(Object proxy) {
 		return proxy != null
-				&& ( proxy instanceof MapProxy || entityDescriptor.getJavaType().getJavaTypeClass().isInstance( proxy ) );
+			&& ( proxy instanceof MapProxy || entityDescriptor.getJavaType().getJavaTypeClass().isInstance( proxy ) );
 	}
 
 	private boolean isExistingEntityInitialized(Object existingEntity) {

hibernate-core/src/test/java/org/hibernate/orm/test/bytecode/enhancement/lazy/HHH_10708/UnexpectedDeleteTest2.java

@@ -8,11 +8,14 @@
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
 
+import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced;
 import org.hibernate.testing.orm.junit.DomainModel;
 import org.hibernate.testing.orm.junit.JiraKey;
+import org.hibernate.testing.orm.junit.ServiceRegistry;
 import org.hibernate.testing.orm.junit.SessionFactory;
 import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.hibernate.testing.orm.junit.Setting;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -33,6 +36,7 @@
 )
 @SessionFactory
 @BytecodeEnhanced
+@ServiceRegistry(settings = @Setting(name = AvailableSettings.ALLOW_REFRESH_DETACHED_ENTITY, value = "true"))
 public class UnexpectedDeleteTest2 {
 
     private Bar myBar;