HHH-19273 Don't trust state of read-only associations for initialization

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/CodeTemplates.java

@@ -176,7 +176,7 @@
 		@Advice.OnMethodEnter
 		static void $$_hibernate_suspendDirtyTracking(
 				@Advice.Argument(0) boolean suspend,
 				@Advice.FieldValue(value = EnhancerConstants.TRACKER_FIELD_NAME, readOnly = false) DirtyTracker $$_hibernate_tracker) {
 			if ( $$_hibernate_tracker == null ) {
 				$$_hibernate_tracker = new SimpleFieldTracker();
 			}
@@ -190,16 +190,18 @@
 				@Advice.Return(readOnly = false) boolean returned,
 				@FieldName String fieldName,
 				@FieldValue Collection<?> collection,
-				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
-			if ( !returned && $$_hibernate_collectionTracker != null ) {
-				final int size = $$_hibernate_collectionTracker.getSize( fieldName );
-				if ( collection == null && size != -1 ) {
+				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker,
+				@AttributeInterceptor PersistentAttributeInterceptor $$_hibernate_attributeInterceptor) {
+			// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
+			if ( !returned && $$_hibernate_collectionTracker != null && ( $$_hibernate_attributeInterceptor == null
+					|| $$_hibernate_attributeInterceptor.isAttributeLoaded( fieldName ) ) ) {
+				if ( collection == null && $$_hibernate_collectionTracker.getSize( fieldName ) != -1 ) {
 					returned = true;
 				}
 				else if ( collection != null ) {
 					// We only check sizes of non-persistent or initialized persistent collections
 					if ( ( !( collection instanceof PersistentCollection ) || ( (PersistentCollection<?>) collection ).wasInitialized() )
-							&& size != collection.size() ) {
+							&& $$_hibernate_collectionTracker.getSize( fieldName ) != collection.size() ) {
 						returned = true;
 					}
 				}
@@ -213,16 +215,18 @@
 				@Advice.Return(readOnly = false) boolean returned,
 				@FieldName String fieldName,
 				@FieldValue Map<?, ?> map,
-				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
-			if ( !returned && $$_hibernate_collectionTracker != null ) {
-				final int size = $$_hibernate_collectionTracker.getSize( fieldName );
-				if ( map == null && size != -1 ) {
+				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker,
+				@AttributeInterceptor PersistentAttributeInterceptor $$_hibernate_attributeInterceptor) {
+			// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
+			if ( !returned && $$_hibernate_collectionTracker != null && ( $$_hibernate_attributeInterceptor == null
+					|| $$_hibernate_attributeInterceptor.isAttributeLoaded( fieldName ) ) ) {
+				if ( map == null && $$_hibernate_collectionTracker.getSize( fieldName ) != -1 ) {
 					returned = true;
 				}
 				else if ( map != null ) {
 					// We only check sizes of non-persistent or initialized persistent collections
 					if ( ( !( map instanceof PersistentCollection ) || ( (PersistentCollection) map ).wasInitialized() )
-							&& size != map.size() ) {
+							&& $$_hibernate_collectionTracker.getSize( fieldName ) != map.size() ) {
 						returned = true;
 					}
 				}
@@ -232,20 +236,22 @@
 
 	static class CollectionGetCollectionFieldDirtyNames {
 		@Advice.OnMethodExit
-		static void $$_hibernate_areCollectionFieldsDirty(
+		static void $$_hibernate_getCollectionFieldDirtyNames(
 				@FieldName String fieldName,
 				@FieldValue Collection<?> collection,
 				@Advice.Argument(0) DirtyTracker tracker,
-				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
-			if ( $$_hibernate_collectionTracker != null ) {
-				final int size = $$_hibernate_collectionTracker.getSize( fieldName );
-				if ( collection == null && size != -1 ) {
+				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker,
+				@AttributeInterceptor PersistentAttributeInterceptor $$_hibernate_attributeInterceptor) {
+			// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
+			if ( $$_hibernate_collectionTracker != null && ( $$_hibernate_attributeInterceptor == null
+					|| $$_hibernate_attributeInterceptor.isAttributeLoaded( fieldName ) ) ) {
+				if ( collection == null && $$_hibernate_collectionTracker.getSize( fieldName ) != -1 ) {
 					tracker.add( fieldName );
 				}
 				else if ( collection != null ) {
 					// We only check sizes of non-persistent or initialized persistent collections
 					if ( ( !( collection instanceof PersistentCollection ) || ( (PersistentCollection<?>) collection ).wasInitialized() )
-							&& size != collection.size() ) {
+							&& $$_hibernate_collectionTracker.getSize( fieldName ) != collection.size() ) {
 						tracker.add( fieldName );
 					}
 				}
@@ -255,20 +261,22 @@
 
 	static class MapGetCollectionFieldDirtyNames {
 		@Advice.OnMethodExit
-		static void $$_hibernate_areCollectionFieldsDirty(
+		static void $$_hibernate_getCollectionFieldDirtyNames(
 				@FieldName String fieldName,
 				@FieldValue Map<?, ?> map,
 				@Advice.Argument(0) DirtyTracker tracker,
-				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
-			if ( $$_hibernate_collectionTracker != null ) {
-				final int size = $$_hibernate_collectionTracker.getSize( fieldName );
-				if ( map == null && size != -1 ) {
+				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker,
+				@AttributeInterceptor PersistentAttributeInterceptor $$_hibernate_attributeInterceptor) {
+			// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
+			if ( $$_hibernate_collectionTracker != null && ( $$_hibernate_attributeInterceptor == null
+					|| $$_hibernate_attributeInterceptor.isAttributeLoaded( fieldName ) ) ) {
+				if ( map == null && $$_hibernate_collectionTracker.getSize( fieldName ) != -1 ) {
 					tracker.add( fieldName );
 				}
 				else if ( map != null ) {
 					// We only check sizes of non-persistent or initialized persistent collections
 					if ( ( !( map instanceof PersistentCollection ) || ( (PersistentCollection<?>) map ).wasInitialized() )
-							&& size != map.size() ) {
+							&& $$_hibernate_collectionTracker.getSize( fieldName ) != map.size() ) {
 						tracker.add( fieldName );
 					}
 				}
@@ -278,16 +286,14 @@
 
 	static class CollectionGetCollectionClearDirtyNames {
 		@Advice.OnMethodExit
-		static void $$_hibernate_clearDirtyCollectionNames(
+		static void $$_hibernate_removeDirtyFields(
 				@FieldName String fieldName,
 				@FieldValue Collection<?> collection,
 				@Advice.Argument(value = 0, readOnly = false) LazyAttributeLoadingInterceptor lazyInterceptor,
 				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
-			if ( lazyInterceptor == null || lazyInterceptor.isAttributeLoaded( fieldName ) ) {
-				if ( collection == null || collection instanceof PersistentCollection && !( (PersistentCollection<?>) collection ).wasInitialized() ) {
-					$$_hibernate_collectionTracker.add( fieldName, -1 );
-				}
-				else {
+			// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
+			if ( ( lazyInterceptor == null || lazyInterceptor.isAttributeLoaded( fieldName ) ) && collection != null ) {
+				if ( !( collection instanceof PersistentCollection ) || ( (PersistentCollection<?>) collection ).wasInitialized() ) {
 					$$_hibernate_collectionTracker.add( fieldName, collection.size() );
 				}
 			}
@@ -296,16 +302,14 @@
 
 	static class MapGetCollectionClearDirtyNames {
 		@Advice.OnMethodExit
-		static void $$_hibernate_clearDirtyCollectionNames(
+		static void $$_hibernate_removeDirtyFields(
 				@FieldName String fieldName,
 				@FieldValue Map<?, ?> map,
 				@Advice.Argument(value = 0, readOnly = false) LazyAttributeLoadingInterceptor lazyInterceptor,
 				@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
-			if ( lazyInterceptor == null || lazyInterceptor.isAttributeLoaded( fieldName ) ) {
-				if ( map == null || map instanceof PersistentCollection && !( (PersistentCollection<?>) map ).wasInitialized() ) {
-					$$_hibernate_collectionTracker.add( fieldName, -1 );
-				}
-				else {
+			// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
+			if ( ( lazyInterceptor == null || lazyInterceptor.isAttributeLoaded( fieldName ) ) && map != null ) {
+				if ( !( map instanceof PersistentCollection ) || ( (PersistentCollection<?>) map ).wasInitialized() ) {
 					$$_hibernate_collectionTracker.add( fieldName, map.size() );
 				}
 			}
@@ -594,6 +598,11 @@
 
 	}
 
+	@Retention(RetentionPolicy.RUNTIME)
+	@interface AttributeInterceptor {
+
+	}
+
 	// mapping to get private field from superclass by calling the enhanced reader, for use when field is not visible
 	static class GetterMapping implements Advice.OffsetMapping {
 

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java

@@ -290,6 +290,25 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
 							.defineMethod( EnhancerConstants.TRACKER_COLLECTION_GET_NAME, constants.TypeCollectionTracker, constants.modifierPUBLIC )
 									.intercept( FieldAccessor.ofField( EnhancerConstants.TRACKER_COLLECTION_NAME ) );
 
+					final Advice.OffsetMapping.Factory<?> attributeInterceptor;
+					if ( enhancementContext.hasLazyLoadableAttributes( managedCtClass ) ) {
+						attributeInterceptor = new Advice.OffsetMapping.ForField.Resolved.Factory<>(
+								CodeTemplates.AttributeInterceptor.class,
+								new FieldDescription.Latent(
+										managedCtClass,
+										EnhancerConstants.INTERCEPTOR_FIELD_NAME,
+										constants.modifierPRIVATE_TRANSIENT,
+										constants.TypePersistentAttributeInterceptor.asGenericType(),
+										Collections.emptyList()
+								)
+						);
+					}
+					else {
+						attributeInterceptor = Advice.OffsetMapping.ForStackManipulation.Factory.of(
+								CodeTemplates.AttributeInterceptor.class,
+								null
+						);
+					}
 					Implementation isDirty = StubMethod.INSTANCE, getDirtyNames = StubMethod.INSTANCE, clearDirtyNames = StubMethod.INSTANCE;
 					for ( AnnotatedFieldDescription collectionField : collectionFields ) {
 						String collectionFieldName = collectionField.getName();
@@ -311,11 +330,13 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
 							isDirty = Advice.withCustomMapping()
 									.bind( CodeTemplates.FieldName.class, collectionFieldName )
 									.bind( CodeTemplates.FieldValue.class, fieldDescription )
+									.bind( attributeInterceptor )
 									.to( adviceIsDirty, constants.adviceLocator )
 									.wrap( isDirty );
 							getDirtyNames = Advice.withCustomMapping()
 									.bind( CodeTemplates.FieldName.class, collectionFieldName )
 									.bind( CodeTemplates.FieldValue.class, fieldDescription )
+									.bind( attributeInterceptor )
 									.to( adviceGetDirtyNames, constants.adviceLocator )
 									.wrap( getDirtyNames );
 							clearDirtyNames = Advice.withCustomMapping()
@@ -330,11 +351,13 @@ private DynamicType.Builder<?> doEnhance(Supplier<DynamicType.Builder<?>> builde
 							isDirty = Advice.withCustomMapping()
 									.bind( CodeTemplates.FieldName.class, collectionFieldName )
 									.bind( CodeTemplates.FieldValue.class, getterMapping )
+									.bind( attributeInterceptor )
 									.to( adviceIsDirty, constants.adviceLocator )
 									.wrap( isDirty );
 							getDirtyNames = Advice.withCustomMapping()
 									.bind( CodeTemplates.FieldName.class, collectionFieldName )
 									.bind( CodeTemplates.FieldValue.class, getterMapping )
+									.bind( attributeInterceptor )
 									.to( adviceGetDirtyNames, constants.adviceLocator )
 									.wrap( getDirtyNames );
 							clearDirtyNames = Advice.withCustomMapping()

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/InlineDirtyCheckerEqualsHelper.java

@@ -24,6 +24,19 @@
 		return Objects.deepEquals( a, b );
 	}
 
+	public static boolean areSame(
+			PersistentAttributeInterceptable persistentAttributeInterceptable,
+			String fieldName,
+			Object a,
+			Object b) {
+		final PersistentAttributeInterceptor persistentAttributeInterceptor = persistentAttributeInterceptable.$$_hibernate_getInterceptor();
+		if ( persistentAttributeInterceptor != null
+				&& !persistentAttributeInterceptor.isAttributeLoaded( fieldName ) ) {
+			return false;
+		}
+		return a == b;
+	}
+
 	public static boolean areEquals(
 			PersistentAttributeInterceptable persistentAttributeInterceptable,
 			String fieldName,

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/InlineDirtyCheckingHandler.java

@@ -5,6 +5,7 @@
 package org.hibernate.bytecode.enhance.internal.bytebuddy;
 
 import java.util.Collection;
+import java.util.Map;
 import java.util.Objects;
 
 import jakarta.persistence.EmbeddedId;
@@ -40,16 +41,19 @@ final class InlineDirtyCheckingHandler implements Implementation, ByteCodeAppend
 
 	private final FieldDescription.InDefinedShape persistentField;
 	private final boolean applyLazyCheck;
+	private final boolean applySamenessCheck;
 
 	private InlineDirtyCheckingHandler(
 			Implementation delegate,
 			TypeDescription managedCtClass,
 			FieldDescription.InDefinedShape persistentField,
-			boolean applyLazyCheck) {
+			boolean applyLazyCheck,
+			boolean applySamenessCheck) {
 		this.delegate = delegate;
 		this.managedCtClass = managedCtClass;
 		this.persistentField = persistentField;
 		this.applyLazyCheck = applyLazyCheck;
+		this.applySamenessCheck = applySamenessCheck;
 	}
 
 	static Implementation wrap(
@@ -63,14 +67,16 @@ static Implementation wrap(
 				implementation = Advice.to( CodeTemplates.CompositeDirtyCheckingHandler.class ).wrap( implementation );
 			}
 			else if ( !persistentField.hasAnnotation( Id.class )
-					&& !persistentField.hasAnnotation( EmbeddedId.class )
-					&& !( persistentField.getType().asErasure().isAssignableTo( Collection.class )
-					&& enhancementContext.isMappedCollection( persistentField ) ) ) {
+					&& !persistentField.hasAnnotation( EmbeddedId.class ) ) {
 				implementation = new InlineDirtyCheckingHandler(
 						implementation,
 						managedCtClass,
 						persistentField.asDefined(),
-						enhancementContext.hasLazyLoadableAttributes( managedCtClass )
+						enhancementContext.hasLazyLoadableAttributes( managedCtClass ),
+						// Also track value changes (object identity) for persistent collection attributes
+						( persistentField.getType().asErasure().isAssignableTo( Collection.class )
+								|| persistentField.getType().asErasure().isAssignableTo( Map.class ) )
+								&& enhancementContext.isMappedCollection( persistentField )
 				);
 			}
 
@@ -158,7 +164,7 @@ public Size apply(
 				methodVisitor.visitMethodInsn(
 						Opcodes.INVOKESTATIC,
 						HELPER_TYPE_NAME,
-						"areEquals",
+						applySamenessCheck ? "areSame" : "areEquals",
 						Type.getMethodDescriptor(
 								Type.BOOLEAN_TYPE,
 								PE_INTERCEPTABLE_TYPE,

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/CollectionPart.java

@@ -90,4 +90,9 @@ default String getPartName() {
 	default ModelPart getInclusionCheckPart() {
 		return this;
 	}
+
+	@Override
+	default boolean isReadOnly() {
+		return getCollectionAttribute().isReadOnly();
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/PluralAttributeMapping.java

@@ -220,4 +220,10 @@ default boolean isPluralAttributeMapping() {
 		return true;
 	}
 
+	@Override
+	default boolean isReadOnly() {
+		return getCollectionDescriptor().getMappedByProperty() != null
+				|| getKeyDescriptor().getKeyPart().isReadOnly();
+	}
+
 }

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/SelectableMappings.java

@@ -40,4 +40,15 @@ default int forEachSelectable(SelectableConsumer consumer) {
 		return forEachSelectable( 0, consumer );
 	}
 
+	default boolean isReadOnly() {
+		final int jdbcTypeCount = getJdbcTypeCount();
+		for ( int i = 0; i < jdbcTypeCount; i++ ) {
+			final var selectableMapping = getSelectable( i );
+			if ( selectableMapping.isInsertable() || selectableMapping.isUpdateable() ) {
+				return false;
+			}
+		}
+		return true;
+	}
+
 }

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -2455,7 +2455,7 @@ public int[] resolveDirtyAttributeIndexes(
 		}
 
 		if ( attributeNames.length != 0 ) {
-			final boolean[] propertyUpdateability = getPropertyUpdateability();
+			final boolean[] propertyCheckability = getPropertyCheckability();
 			if ( superMappingType == null ) {
 				/*
 						Sort attribute names so that we can traverse mappings efficiently
@@ -2482,7 +2482,7 @@ class ChildEntity extends SuperEntity {
 					final String attributeName = attributeMapping.getAttributeName();
 					if ( isPrefix( attributeMapping, attributeNames[index] ) ) {
 						final int position = attributeMapping.getStateArrayPosition();
-						if ( propertyUpdateability[position] && !fields.contains( position ) ) {
+						if ( propertyCheckability[position] && !fields.contains( position ) ) {
 							fields.add( position );
 						}
 						index++;
@@ -2506,7 +2506,7 @@ class ChildEntity extends SuperEntity {
 			else {
 				for ( String attributeName : attributeNames ) {
 					final Integer index = getPropertyIndexOrNull( attributeName );
-					if ( index != null && propertyUpdateability[index] && !fields.contains( index ) ) {
+					if ( index != null && propertyCheckability[index] && !fields.contains( index ) ) {
 						fields.add( index );
 					}
 				}

hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/AbstractImmediateCollectionInitializer.java

@@ -37,6 +37,7 @@
 public abstract class AbstractImmediateCollectionInitializer<Data extends AbstractImmediateCollectionInitializer.ImmediateCollectionInitializerData>
 		extends AbstractCollectionInitializer<Data> implements BiConsumer<Data, List<Object>> {
 
+	private final boolean isReadOnly;
 	/**
 	 * refers to the rows entry in the collection.  null indicates that the collection is empty
 	 */
@@ -82,6 +83,7 @@ public AbstractImmediateCollectionInitializer(
 				collectionKeyResult == collectionValueKeyResult
 						? null
 						: collectionValueKeyResult.createResultAssembler( this, creationState );
+		this.isReadOnly = collectionAttributeMapping.isReadOnly();
 	}
 
 	@Override
@@ -319,7 +321,13 @@ protected void setMissing(Data data) {
 	public void resolveInstance(Object instance, Data data) {
 		assert data.getState() == State.UNINITIALIZED || instance == data.getCollectionInstance();
 		if ( instance == null ) {
-			setMissing( data );
+			if ( isReadOnly ) {
+				// When the mapping is read-only, we can't trust the state of the persistence context
+				resolveKey( data );
+			}
+			else {
+				setMissing( data );
+			}
 		}
 		else {
 			final var rowProcessingState = data.getRowProcessingState();