From 5452f9ee3466fc36b33016e802dabd5c2185f8ac Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Mon, 18 Aug 2025 10:04:03 +0200 Subject: [PATCH 1/2] HHH-19605 Add test for issue --- .../test/dirtiness/SessionIsDirtyTests.java | 215 ++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/dirtiness/SessionIsDirtyTests.java diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/dirtiness/SessionIsDirtyTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/dirtiness/SessionIsDirtyTests.java new file mode 100644 index 000000000000..25257528c9df --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/dirtiness/SessionIsDirtyTests.java @@ -0,0 +1,215 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.dirtiness; + +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import org.hibernate.Hibernate; +import org.hibernate.annotations.Cache; +import org.hibernate.annotations.CacheConcurrencyStrategy; +import org.hibernate.cache.spi.CacheImplementor; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.Jira; +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.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; + +@DomainModel(annotatedClasses = { + SessionIsDirtyTests.EntityA.class, + SessionIsDirtyTests.EntityB.class, + SessionIsDirtyTests.EntityC.class, +}) +@ServiceRegistry(settings = { + @Setting(name = AvailableSettings.DEFAULT_BATCH_FETCH_SIZE, value = "5"), + @Setting(name = AvailableSettings.USE_SECOND_LEVEL_CACHE, value = "true"), +}) +@SessionFactory +@Jira("https://hibernate.atlassian.net/browse/HHH-19605") +public class SessionIsDirtyTests { + @Test + public void testBatchAndCacheDirtiness(SessionFactoryScope scope) { + final CacheImplementor cache = scope.getSessionFactory().getCache(); + cache.evictAllRegions(); + scope.inTransaction( session -> { + final List resultList = session.createSelectionQuery( + "select a from EntityA a order by a.id", + EntityA.class + ).getResultList(); + assertThat( session.isDirty() ).isFalse(); + + assertThat( resultList ).hasSize( 2 ); + final EntityA entityA1 = resultList.get( 0 ); + assertThat( entityA1.getId() ).isEqualTo( 1L ); + assertThat( entityA1.getName() ).isEqualTo( "A1" ); + assertThat( entityA1.getEntityB() ).isNull(); + + final EntityA entityA2 = resultList.get( 1 ); + assertThat( entityA2.getId() ).isEqualTo( 2L ); + assertThat( entityA2.getName() ).isEqualTo( "A2" ); + assertThat( entityA2.getEntityB() ).isNotNull(); + assertThat( entityA2.getEntityB().getEntityA() ).isSameAs( entityA1 ); + + entityA2.getEntityB().setName( "B1 updated" ); + assertThat( session.isDirty() ).isTrue(); + } ); + } + + @Test + public void testLazyAssociationDirtiness(SessionFactoryScope scope) { + scope.inTransaction( session -> { + final List resultList = session.createSelectionQuery( + "select c from EntityC c order by c.id", + EntityC.class + ).getResultList(); + assertThat( session.isDirty() ).isFalse(); + + assertThat( resultList ).hasSize( 1 ); + final EntityC entityC = resultList.get( 0 ); + assertThat( entityC.getId() ).isEqualTo( 1L ); + assertThat( entityC.getName() ).isEqualTo( "C1" ); + assertThat( Hibernate.isInitialized( entityC.getEntityB() ) ).isFalse(); + + entityC.getEntityB().setName( "B1 lazy updated" ); + assertThat( session.isDirty() ).isTrue(); + } ); + } + + @BeforeAll + public void setUp(SessionFactoryScope scope) { + scope.inTransaction( session -> { + final EntityA entityA1 = new EntityA( 1L, "A1" ); + final EntityA entityA2 = new EntityA( 2L, "A2" ); + final EntityB entityB = new EntityB( 1L, "B1" ); + entityB.entityA = entityA1; + entityA2.entityB = entityB; + session.persist( entityA1 ); + session.persist( entityA2 ); + session.persist( entityB ); + + final EntityC entityC = new EntityC( 1L, "C1" ); + entityC.entityB = entityB; + session.persist( entityC ); + } ); + } + + @AfterAll + public void tearDown(SessionFactoryScope scope) { + scope.getSessionFactory().getSchemaManager().truncateMappedObjects(); + } + + @Entity(name = "EntityA") + @Cache(usage = CacheConcurrencyStrategy.READ_WRITE) + static class EntityA { + @Id + Long id; + + String name; + + @ManyToOne + @JoinColumn(name = "entity_b") + EntityB entityB; + + public EntityA() { + } + + public EntityA(Long id, String name) { + this.id = id; + this.name = name; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + public EntityB getEntityB() { + return entityB; + } + } + + @Entity(name = "EntityB") + @Cache(usage = CacheConcurrencyStrategy.READ_WRITE) + static class EntityB { + @Id + Long id; + + String name; + + @ManyToOne + @JoinColumn(name = "entity_a") + EntityA entityA; + + public EntityB() { + } + + public EntityB(Long id, String name) { + this.id = id; + this.name = name; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + public EntityA getEntityA() { + return entityA; + } + + public void setName(String name) { + this.name = name; + } + } + + @Entity(name = "EntityC") + static class EntityC { + @Id + Long id; + + String name; + + @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "entity_b") + EntityB entityB; + + public EntityC() { + } + + public EntityC(Long id, String name) { + this.id = id; + this.name = name; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + public EntityB getEntityB() { + return entityB; + } + } +} From 2ee7d1eae9990b56101e5637c9be49ad37d7e3b4 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Wed, 13 Aug 2025 14:44:43 +0200 Subject: [PATCH 2/2] HHH-19605 Fix entity dirtiness logic when dealing with proxies --- .../engine/internal/EntityEntryImpl.java | 50 ++++++------------- .../DefaultDirtyCheckEventListener.java | 27 ++++------ 2 files changed, 26 insertions(+), 51 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/EntityEntryImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/EntityEntryImpl.java index 580bb05f998c..80588cdb65de 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/EntityEntryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/EntityEntryImpl.java @@ -10,7 +10,6 @@ import java.io.Serializable; import org.hibernate.AssertionFailure; -import org.hibernate.CustomEntityDirtinessStrategy; import org.hibernate.HibernateException; import org.hibernate.LockMode; import org.hibernate.UnsupportedLockAttemptException; @@ -21,7 +20,6 @@ import org.hibernate.engine.spi.EntityKey; import org.hibernate.engine.spi.ManagedEntity; import org.hibernate.engine.spi.PersistenceContext; -import org.hibernate.engine.spi.PersistentAttributeInterceptor; import org.hibernate.engine.spi.SelfDirtinessTracker; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SessionImplementor; @@ -41,10 +39,8 @@ import static org.hibernate.engine.internal.EntityEntryImpl.EnumState.PREVIOUS_STATUS; import static org.hibernate.engine.internal.EntityEntryImpl.EnumState.STATUS; import static org.hibernate.engine.internal.ManagedTypeHelper.asManagedEntity; -import static org.hibernate.engine.internal.ManagedTypeHelper.asPersistentAttributeInterceptable; +import static org.hibernate.engine.internal.ManagedTypeHelper.asPersistentAttributeInterceptableOrNull; import static org.hibernate.engine.internal.ManagedTypeHelper.asSelfDirtinessTracker; -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.ManagedTypeHelper.processIfManagedEntity; import static org.hibernate.engine.internal.ManagedTypeHelper.processIfSelfDirtinessTracker; @@ -56,7 +52,6 @@ import static org.hibernate.engine.spi.Status.SAVING; import static org.hibernate.internal.util.StringHelper.nullIfEmpty; import static org.hibernate.pretty.MessageHelper.infoString; -import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer; /** * A base implementation of {@link EntityEntry}. @@ -390,46 +385,31 @@ private boolean isUnequivocallyNonDirty(Object entity) { } private boolean isNonDirtyViaCustomStrategy(Object entity) { - if ( isPersistentAttributeInterceptable( entity ) ) { - final PersistentAttributeInterceptor interceptor = - asPersistentAttributeInterceptable( entity ).$$_hibernate_getInterceptor(); - if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) { + final var interceptable = asPersistentAttributeInterceptableOrNull( entity ); + if ( interceptable != null ) { + if ( interceptable.$$_hibernate_getInterceptor() instanceof EnhancementAsProxyLazinessInterceptor interceptor + && !interceptor.isInitialized() ) { // we never have to check an uninitialized proxy return true; } } - - final SessionImplementor session = (SessionImplementor) getPersistenceContext().getSession(); - final CustomEntityDirtinessStrategy customEntityDirtinessStrategy = - session.getFactory().getCustomEntityDirtinessStrategy(); + final var session = (SessionImplementor) getPersistenceContext().getSession(); + final var customEntityDirtinessStrategy = session.getFactory().getCustomEntityDirtinessStrategy(); return customEntityDirtinessStrategy.canDirtyCheck( entity, persister, session ) && !customEntityDirtinessStrategy.isDirty( entity, persister, session ); } private boolean isNonDirtyViaTracker(Object entity) { - final boolean uninitializedProxy; - if ( isPersistentAttributeInterceptable( entity ) ) { - final PersistentAttributeInterceptor interceptor = - asPersistentAttributeInterceptable( entity ).$$_hibernate_getInterceptor(); - if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor lazinessInterceptor ) { - return !lazinessInterceptor.hasWrittenFieldNames(); - } - else { - uninitializedProxy = false; + final var interceptable = asPersistentAttributeInterceptableOrNull( entity ); + if ( interceptable != null ) { + if ( interceptable.$$_hibernate_getInterceptor() instanceof EnhancementAsProxyLazinessInterceptor interceptor ) { + return !interceptor.hasWrittenFieldNames(); } } - else if ( isHibernateProxy( entity ) ) { - uninitializedProxy = extractLazyInitializer( entity ).isUninitialized(); - } - else { - uninitializedProxy = false; - } - // we never have to check an uninitialized proxy - return uninitializedProxy - || !persister.hasCollections() - && !persister.hasMutableProperties() - && !asSelfDirtinessTracker( entity ).$$_hibernate_hasDirtyAttributes() - && asManagedEntity( entity ).$$_hibernate_useTracker(); + return !persister.hasCollections() + && !persister.hasMutableProperties() + && asManagedEntity( entity ).$$_hibernate_useTracker() + && !asSelfDirtinessTracker( entity ).$$_hibernate_hasDirtyAttributes(); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDirtyCheckEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDirtyCheckEventListener.java index eddb40ab850a..f2cd156f2cb6 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDirtyCheckEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDirtyCheckEventListener.java @@ -8,8 +8,6 @@ import org.hibernate.HibernateException; import org.hibernate.collection.spi.PersistentCollection; import org.hibernate.engine.spi.EntityEntry; -import org.hibernate.engine.spi.EntityHolder; -import org.hibernate.engine.spi.PersistenceContext; import org.hibernate.engine.spi.Status; import org.hibernate.event.spi.DirtyCheckEvent; import org.hibernate.event.spi.DirtyCheckEventListener; @@ -38,15 +36,13 @@ public class DefaultDirtyCheckEventListener implements DirtyCheckEventListener { @Override public void onDirtyCheck(DirtyCheckEvent event) throws HibernateException { - final EventSource session = event.getSession(); - final PersistenceContext persistenceContext = session.getPersistenceContext(); - final var holdersByKey = persistenceContext.getEntityHoldersByKey(); - if ( holdersByKey != null ) { - for ( var entry : holdersByKey.entrySet() ) { - if ( isEntityDirty( entry.getValue(), session ) ) { - event.setDirty( true ); - return; - } + final var session = event.getSession(); + final var persistenceContext = session.getPersistenceContext(); + final var entityEntries = persistenceContext.reentrantSafeEntityEntries(); + for ( var me : entityEntries ) { + if ( isEntityDirty( me.getKey(), me.getValue(), session ) ) { + event.setDirty( true ); + return; } } final var entriesByCollection = persistenceContext.getCollectionEntries(); @@ -60,20 +56,19 @@ public void onDirtyCheck(DirtyCheckEvent event) throws HibernateException { } } - private static boolean isEntityDirty(EntityHolder holder, EventSource session) { - final EntityEntry entityEntry = holder.getEntityEntry(); + private static boolean isEntityDirty(Object entity, EntityEntry entityEntry, EventSource session) { final Status status = entityEntry.getStatus(); return switch ( status ) { case GONE, READ_ONLY -> false; case DELETED -> true; - case MANAGED -> isManagedEntityDirty( holder.getManagedObject(), holder.getDescriptor(), entityEntry, session ); + case MANAGED -> isManagedEntityDirty( entity, entityEntry, session ); case SAVING, LOADING -> throw new AssertionFailure( "Unexpected status: " + status ); }; } - private static boolean isManagedEntityDirty( - Object entity, EntityPersister descriptor, EntityEntry entityEntry, EventSource session) { + private static boolean isManagedEntityDirty(Object entity, EntityEntry entityEntry, EventSource session) { if ( entityEntry.requiresDirtyCheck( entity ) ) { // takes into account CustomEntityDirtinessStrategy + final EntityPersister descriptor = entityEntry.getPersister(); final Object[] propertyValues = entityEntry.getStatus() == Status.DELETED ? entityEntry.getDeletedState()