HHH-14329 consider mutable types always as potentially dirty when usi…

…ng DirtinessTracker

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

@@ -411,7 +411,7 @@ private List<AnnotatedFieldDescription> collectCollectionFields(TypeDescription
 				continue;
 			}
 			AnnotatedFieldDescription annotatedField = new AnnotatedFieldDescription( enhancementContext, ctField );
-			if ( enhancementContext.isPersistentField( annotatedField ) && !enhancementContext.isMappedCollection( annotatedField ) ) {
+			if ( enhancementContext.isPersistentField( annotatedField ) && enhancementContext.isMappedCollection( annotatedField ) ) {
 				if ( ctField.getType().asErasure().isAssignableTo( Collection.class ) || ctField.getType().asErasure().isAssignableTo( Map.class ) ) {
 					collectionList.add( annotatedField );
 				}
@@ -441,7 +441,7 @@ private Collection<AnnotatedFieldDescription> collectInheritCollectionFields(Typ
 		for ( FieldDescription ctField : managedCtSuperclass.getDeclaredFields() ) {
 			if ( !Modifier.isStatic( ctField.getModifiers() ) ) {
 				AnnotatedFieldDescription annotatedField = new AnnotatedFieldDescription( enhancementContext, ctField );
-				if ( enhancementContext.isPersistentField( annotatedField ) && !enhancementContext.isMappedCollection( annotatedField ) ) {
+				if ( enhancementContext.isPersistentField( annotatedField ) && enhancementContext.isMappedCollection( annotatedField ) ) {
 					if ( ctField.getType().asErasure().isAssignableTo( Collection.class ) || ctField.getType().asErasure().isAssignableTo( Map.class ) ) {
 						collectionList.add( annotatedField );
 					}

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/javassist/EntityEnhancer.java

@@ -283,7 +283,7 @@ private List<CtField> collectCollectionFields(CtClass managedCtClass) {
 			if ( Modifier.isStatic( ctField.getModifiers() ) || ctField.getName().startsWith( "$$_hibernate_" ) ) {
 				continue;
 			}
-			if ( enhancementContext.isPersistentField( ctField ) && !enhancementContext.isMappedCollection( ctField ) ) {
+			if ( enhancementContext.isPersistentField( ctField ) && enhancementContext.isMappedCollection( ctField ) ) {
 				if ( PersistentAttributesHelper.isAssignable( ctField, Collection.class.getName() ) ||
 						PersistentAttributesHelper.isAssignable( ctField, Map.class.getName() ) ) {
 					collectionList.add( ctField );
@@ -314,7 +314,7 @@ private Collection<CtField> collectInheritCollectionFields(CtClass managedCtClas
 
 			for ( CtField ctField : managedCtSuperclass.getDeclaredFields() ) {
 				if ( !Modifier.isStatic( ctField.getModifiers() ) ) {
-					if ( enhancementContext.isPersistentField( ctField ) && !enhancementContext.isMappedCollection( ctField ) ) {
+					if ( enhancementContext.isPersistentField( ctField ) && enhancementContext.isMappedCollection( ctField ) ) {
 						if ( PersistentAttributesHelper.isAssignable( ctField, Collection.class.getName() ) ||
 								PersistentAttributesHelper.isAssignable( ctField, Map.class.getName() ) ) {
 							collectionList.add( ctField );

hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/DefaultEnhancementContext.java

@@ -6,6 +6,8 @@
  */
 package org.hibernate.bytecode.enhance.spi;
 
+import javax.persistence.Basic;
+import javax.persistence.Convert;
 import javax.persistence.ElementCollection;
 import javax.persistence.Embeddable;
 import javax.persistence.Entity;
@@ -106,7 +108,13 @@ public boolean isPersistentField(UnloadedField ctField) {
 	 */
 	@Override
 	public boolean isMappedCollection(UnloadedField field) {
-		return field.hasAnnotation( OneToMany.class ) || field.hasAnnotation( ManyToMany.class )  || field.hasAnnotation( ElementCollection.class );
+		// If the collection is definitely a plural attribute, we respect that
+		if (field.hasAnnotation( OneToMany.class ) || field.hasAnnotation( ManyToMany.class ) || field.hasAnnotation( ElementCollection.class )) {
+			return true;
+		}
+		// But a collection might be treated like a singular attribute if it is annotated with `@Basic`
+		// If no annotations are given though, a collection is treated like a OneToMany
+		return !field.hasAnnotation( Basic.class );
 	}
 
 	/**

hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java

@@ -33,6 +33,7 @@
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.persister.entity.UniqueKeyLoadable;
 import org.hibernate.pretty.MessageHelper;
+import org.hibernate.proxy.HibernateProxy;
 
 /**
  * A base implementation of EntityEntry
@@ -346,7 +347,24 @@ public boolean requiresDirtyCheck(Object entity) {
 	@SuppressWarnings( {"SimplifiableIfStatement"})
 	private boolean isUnequivocallyNonDirty(Object entity) {
 		if ( entity instanceof SelfDirtinessTracker ) {
-			return ! persister.hasCollections() && ! ( (SelfDirtinessTracker) entity ).$$_hibernate_hasDirtyAttributes();
+			boolean uninitializedProxy = false;
+			if ( entity instanceof PersistentAttributeInterceptable ) {
+				final PersistentAttributeInterceptable interceptable = (PersistentAttributeInterceptable) entity;
+				final PersistentAttributeInterceptor interceptor = interceptable.$$_hibernate_getInterceptor();
+				if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) {
+					EnhancementAsProxyLazinessInterceptor enhancementAsProxyLazinessInterceptor = (EnhancementAsProxyLazinessInterceptor) interceptor;
+					// When a proxy has dirty attributes, we have to treat it like a normal entity to flush changes
+					uninitializedProxy = !enhancementAsProxyLazinessInterceptor.isInitialized() && !( (SelfDirtinessTracker) entity ).$$_hibernate_hasDirtyAttributes();
+				}
+			}
+			else if ( entity instanceof HibernateProxy ) {
+				uninitializedProxy = ( (HibernateProxy) entity ).getHibernateLazyInitializer()
+						.isUninitialized();
+			}
+			// we never have to check an uninitialized proxy
+			return uninitializedProxy || !persister.hasCollections()
+					&& !persister.hasMutableProperties()
+					&& !( (SelfDirtinessTracker) entity ).$$_hibernate_hasDirtyAttributes();
 		}
 
 		if ( entity instanceof PersistentAttributeInterceptable ) {

hibernate-core/src/main/java/org/hibernate/event/internal/DefaultFlushEntityEventListener.java

@@ -22,6 +22,7 @@
 import org.hibernate.engine.internal.Versioning;
 import org.hibernate.engine.spi.EntityEntry;
 import org.hibernate.engine.spi.EntityKey;
+import org.hibernate.engine.spi.ManagedEntity;
 import org.hibernate.engine.spi.PersistenceContext;
 import org.hibernate.engine.spi.PersistentAttributeInterceptable;
 import org.hibernate.engine.spi.PersistentAttributeInterceptor;
@@ -39,6 +40,7 @@
 import org.hibernate.metadata.ClassMetadata;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.pretty.MessageHelper;
+import org.hibernate.proxy.HibernateProxy;
 import org.hibernate.stat.spi.StatisticsImplementor;
 import org.hibernate.type.Type;
 
@@ -525,18 +527,13 @@ protected void dirtyCheck(final FlushEntityEvent event) throws HibernateExceptio
 
 		if ( dirtyProperties == null ) {
 			if ( entity instanceof SelfDirtinessTracker ) {
-				if ( ( (SelfDirtinessTracker) entity ).$$_hibernate_hasDirtyAttributes() ) {
-					int[] dirty = persister.resolveAttributeIndexes( ( (SelfDirtinessTracker) entity ).$$_hibernate_getDirtyAttributes() );
-
-					// HHH-12051 - filter non-updatable attributes
-					// TODO: add Updatability to EnhancementContext and skip dirty tracking of those attributes
-					int count = 0;
-					for ( int i : dirty ) {
-						if ( persister.getPropertyUpdateability()[i] ) {
-							dirty[count++] = i;
-						}
-					}
-					dirtyProperties = count == 0 ? ArrayHelper.EMPTY_INT_ARRAY : count == dirty.length ? dirty : Arrays.copyOf( dirty, count );
+				if ( ( (SelfDirtinessTracker) entity ).$$_hibernate_hasDirtyAttributes() || persister.hasMutableProperties() ) {
+					dirtyProperties = persister.resolveDirtyAttributeIndexes(
+							values,
+							loadedState,
+							( (SelfDirtinessTracker) entity ).$$_hibernate_getDirtyAttributes(),
+							session
+					);
 				}
 				else {
 					dirtyProperties = ArrayHelper.EMPTY_INT_ARRAY;

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

@@ -12,6 +12,7 @@
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.BitSet;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
@@ -67,7 +68,6 @@
 import org.hibernate.engine.jdbc.spi.JdbcServices;
 import org.hibernate.engine.spi.CachedNaturalIdValueSource;
 import org.hibernate.engine.spi.CascadeStyle;
-import org.hibernate.engine.spi.CascadingAction;
 import org.hibernate.engine.spi.CascadingActions;
 import org.hibernate.engine.spi.CollectionKey;
 import org.hibernate.engine.spi.EntityEntry;
@@ -81,6 +81,7 @@
 import org.hibernate.engine.spi.PersistentAttributeInterceptable;
 import org.hibernate.engine.spi.PersistentAttributeInterceptor;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.engine.spi.ValueInclusion;
 import org.hibernate.event.spi.EventSource;
@@ -104,7 +105,6 @@
 import org.hibernate.loader.entity.BatchingEntityLoaderBuilder;
 import org.hibernate.loader.entity.CacheEntityLoaderHelper;
 import org.hibernate.loader.entity.CascadeEntityLoader;
-import org.hibernate.loader.entity.plan.DynamicBatchingEntityLoaderBuilder;
 import org.hibernate.loader.entity.EntityLoader;
 import org.hibernate.loader.entity.UniqueEntityLoader;
 import org.hibernate.loader.entity.plan.MultiEntityLoadingSupport;
@@ -2261,6 +2261,52 @@ public int[] resolveAttributeIndexes(String[] attributeNames) {
 		return Arrays.copyOf( fields, counter );
 	}
 
+	@Override
+	public int[] resolveDirtyAttributeIndexes(
+			final Object[] currentState,
+			final Object[] previousState,
+			final String[] attributeNames,
+			final SessionImplementor session) {
+		final BitSet mutablePropertiesIndexes = entityMetamodel.getMutablePropertiesIndexes();
+		final int estimatedSize = attributeNames == null ? 0 : attributeNames.length + mutablePropertiesIndexes.cardinality();
+		final List<Integer> fields = new ArrayList<>( estimatedSize );
+		if ( estimatedSize == 0 ) {
+			return ArrayHelper.EMPTY_INT_ARRAY;
+		}
+		if ( !mutablePropertiesIndexes.isEmpty() ) {
+			// We have to check the state for "mutable" properties as dirty tracking isn't aware of mutable types
+			final Type[] propertyTypes = entityMetamodel.getPropertyTypes();
+			final boolean[] propertyCheckability = entityMetamodel.getPropertyCheckability();
+			mutablePropertiesIndexes.stream().forEach( i -> {
+				// This is kindly borrowed from org.hibernate.type.TypeHelper.findDirty
+				final boolean dirty = currentState[i] != LazyPropertyInitializer.UNFETCHED_PROPERTY &&
+						( previousState[i] == LazyPropertyInitializer.UNFETCHED_PROPERTY ||
+								( propertyCheckability[i]
+										&& propertyTypes[i].isDirty(
+										previousState[i],
+										currentState[i],
+										propertyColumnUpdateable[i],
+										session
+								) ) );
+				if ( dirty ) {
+					fields.add( i );
+				}
+			} );
+		}
+
+		if ( attributeNames != null ) {
+			final boolean[] propertyUpdateability = entityMetamodel.getPropertyUpdateability();
+			for ( String attributeName : attributeNames ) {
+				final Integer index = entityMetamodel.getPropertyIndexOrNull( attributeName );
+				if ( index != null && propertyUpdateability[index] && !fields.contains( index ) ) {
+					fields.add( index );
+				}
+			}
+		}
+
+		return ArrayHelper.toIntArray( fields );
+	}
+
 	protected String[] getSubclassPropertySubclassNameClosure() {
 		return subclassPropertySubclassNameClosure;
 	}

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

@@ -17,14 +17,14 @@
 import org.hibernate.MappingException;
 import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementAsProxyLazinessInterceptor;
 import org.hibernate.bytecode.spi.BytecodeEnhancementMetadata;
-import org.hibernate.bytecode.spi.NotInstrumentedException;
 import org.hibernate.cache.spi.access.EntityDataAccess;
 import org.hibernate.cache.spi.access.NaturalIdDataAccess;
 import org.hibernate.cache.spi.entry.CacheEntry;
 import org.hibernate.cache.spi.entry.CacheEntryStructure;
 import org.hibernate.engine.spi.CascadeStyle;
 import org.hibernate.engine.spi.EntityEntryFactory;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.engine.spi.ValueInclusion;
 import org.hibernate.id.IdentifierGenerator;
@@ -840,6 +840,25 @@ default BytecodeEnhancementMetadata getBytecodeEnhancementMetadata() {
 	 */
 	int[] resolveAttributeIndexes(String[] attributeNames);
 
+	/**
+	 * Like {@link #resolveAttributeIndexes(String[])} but also always returns mutable attributes
+	 *
+	 *
+	 * @param values
+	 * @param loadedState
+	 * @param attributeNames Array of names to be resolved
+	 *
+	 * @param session
+	 * @return A set of unique indexes of the attribute names found in the metamodel
+	 */
+	default int[] resolveDirtyAttributeIndexes(
+			Object[] values,
+			Object[] loadedState,
+			String[] attributeNames,
+			SessionImplementor session) {
+		return resolveAttributeIndexes( attributeNames );
+	}
+
 	boolean canUseReferenceCacheEntries();
 
 	/**

hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java

@@ -8,6 +8,7 @@
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.BitSet;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -44,6 +45,7 @@
 import org.hibernate.tuple.ValueGeneration;
 import org.hibernate.tuple.ValueGenerator;
 import org.hibernate.type.AssociationType;
+import org.hibernate.type.ComponentType;
 import org.hibernate.type.CompositeType;
 import org.hibernate.type.EntityType;
 import org.hibernate.type.Type;
@@ -97,7 +99,7 @@ public class EntityMetamodel implements Serializable {
 
 	private final Map<String, Integer> propertyIndexes = new HashMap<>();
 	private final boolean hasCollections;
-	private final boolean hasMutableProperties;
+	private final BitSet mutablePropertiesIndexes;
 	private final boolean hasLazyProperties;
 	private final boolean hasNonIdentifierPropertyNamedId;
 
@@ -208,7 +210,7 @@ public EntityMetamodel(
 		int tempVersionProperty = NO_VERSION_INDX;
 		boolean foundCascade = false;
 		boolean foundCollection = false;
-		boolean foundMutable = false;
+		BitSet mutableIndexes = new BitSet();
 		boolean foundNonIdentifierPropertyNamedId = false;
 		boolean foundUpdateableNaturalIdProperty = false;
 
@@ -318,8 +320,9 @@ else if ( timing == GenerationTiming.ALWAYS ) {
 				foundCollection = true;
 			}
 
-			if ( propertyTypes[i].isMutable() && propertyCheckability[i] ) {
-				foundMutable = true;
+			// Component types are dirty tracked as well so they are not exactly mutable for the "maybeDirty" check
+			if ( propertyTypes[i].isMutable() && propertyCheckability[i] && !( propertyTypes[i] instanceof ComponentType ) ) {
+				mutableIndexes.set( i );
 			}
 
 			mapPropertyToIndex(prop, i);
@@ -395,7 +398,7 @@ else if ( timing == GenerationTiming.ALWAYS ) {
 		}
 
 		hasCollections = foundCollection;
-		hasMutableProperties = foundMutable;
+		mutablePropertiesIndexes = mutableIndexes;
 
 		iter = persistentClass.getSubclassIterator();
 		final Set<String> subclassEntityNamesLocal = new HashSet<>();
@@ -890,7 +893,11 @@ public boolean hasCollections() {
 	}
 
 	public boolean hasMutableProperties() {
-		return hasMutableProperties;
+		return !mutablePropertiesIndexes.isEmpty();
+	}
+
+	public BitSet getMutablePropertiesIndexes() {
+		return mutablePropertiesIndexes;
 	}
 
 	public boolean hasNonIdentifierPropertyNamedId() {