From 2669848ca448ee14660350d707f60b23fd46eea6 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Fri, 13 Nov 2020 16:10:00 +0100 Subject: [PATCH] HHH-14329 consider mutable types always as potentially dirty when using DirtinessTracker --- .../internal/bytebuddy/EnhancerImpl.java | 4 +- .../internal/javassist/EntityEnhancer.java | 4 +- .../spi/DefaultEnhancementContext.java | 10 +++- .../engine/internal/AbstractEntityEntry.java | 20 +++++++- .../DefaultFlushEntityEventListener.java | 21 ++++---- .../entity/AbstractEntityPersister.java | 50 ++++++++++++++++++- .../persister/entity/EntityPersister.java | 21 +++++++- .../tuple/entity/EntityMetamodel.java | 19 ++++--- 8 files changed, 122 insertions(+), 27 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java index 4de4c6df939b..d142264bd9c7 100644 --- a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java @@ -411,7 +411,7 @@ private List 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 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 ); } diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/javassist/EntityEnhancer.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/javassist/EntityEnhancer.java index f55ac6d3a86e..5da39fb16acf 100644 --- a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/javassist/EntityEnhancer.java +++ b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/javassist/EntityEnhancer.java @@ -283,7 +283,7 @@ private List 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 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 ); diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/DefaultEnhancementContext.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/DefaultEnhancementContext.java index 233685576ab1..861515bcc449 100644 --- a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/DefaultEnhancementContext.java +++ b/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 ); } /** diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java index e2cb7db1db75..11a201baef1e 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java +++ b/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 ) { diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultFlushEntityEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultFlushEntityEventListener.java index ced996e913f2..21c97adf8109 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultFlushEntityEventListener.java +++ b/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; diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 4224acc76fc1..2891eb722a55 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/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 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; } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java index e5195da47d5e..2829adfe8d95 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java @@ -17,7 +17,6 @@ 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; @@ -25,6 +24,7 @@ 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(); /** diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java b/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java index 64501db568b4..61564422c0f4 100644 --- a/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java +++ b/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 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 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() {