Skip to content

HHH-19605 Session.isDirty might return true when batch fetching entity with CacheConcurrencyStrategy.READ_WRITE #10777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,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;
Expand All @@ -52,7 +50,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}.
Expand Down Expand Up @@ -385,43 +382,31 @@ private boolean isUnequivocallyNonDirty(Object entity) {
}

private boolean isNonDirtyViaCustomStrategy(Object entity) {
if ( isPersistentAttributeInterceptable( entity ) ) {
if ( asPersistentAttributeInterceptable( entity ).$$_hibernate_getInterceptor()
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 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 ) ) {
if ( asPersistentAttributeInterceptable( entity ).$$_hibernate_getInterceptor()
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +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.Status;
import org.hibernate.event.spi.DirtyCheckEvent;
import org.hibernate.event.spi.DirtyCheckEventListener;
Expand All @@ -17,6 +16,7 @@
import org.hibernate.persister.entity.EntityPersister;



/**
* Determines if the current session holds modified state which
* would be synchronized with the database if the session were
Expand All @@ -39,13 +39,11 @@ public class DefaultDirtyCheckEventListener implements DirtyCheckEventListener {
public void onDirtyCheck(DirtyCheckEvent event) throws HibernateException {
final var session = event.getSession();
final var 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 entityEntries = persistenceContext.reentrantSafeEntityEntries();
for ( var me : entityEntries ) {
if ( isEntityDirty( me.getKey(), me.getValue(), session ) ) {
event.setDirty( true );
return;
}
}
final var entriesByCollection = persistenceContext.getCollectionEntries();
Expand All @@ -59,20 +57,19 @@ public void onDirtyCheck(DirtyCheckEvent event) throws HibernateException {
}
}

private static boolean isEntityDirty(EntityHolder holder, EventSource session) {
final var 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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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<EntityA> 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<EntityC> 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;
}
}
}