diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java index 0b700e9f34ba..3ce204e9662f 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -1046,6 +1046,12 @@ public Object loadFromSecondLevelCache( final Object entity = CacheLoadHelper.loadFromSecondLevelCache( this, instanceToLoad, lockMode, persister, entityKey ); if ( entity != null ) { + final var holder = + getPersistenceContextInternal().getEntityHolder( entityKey ); + if ( holder != null + && holder.getEntityEntry() == null ) { + return entity; + } final Object id = entityKey.getIdentifierValue(); final var postLoadEvent = makePostLoadEvent( persister, id, entity ); eventListenerGroups.eventListenerGroup_POST_LOAD diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractMultiIdEntityLoader.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractMultiIdEntityLoader.java index 4dcc86819820..90a08ddba417 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractMultiIdEntityLoader.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractMultiIdEntityLoader.java @@ -246,10 +246,20 @@ private boolean isLoadFromCaches( // look for it in the second-level cache final Object entity = loadFromSecondLevelCache( entityKey, lockOptions, session ); + final var persistenceContext = session.getPersistenceContextInternal(); if ( entity != null ) { - results.add( i, entity ); + results.add( i, persistenceContext.proxyFor( getLoadable().getEntityPersister(), entityKey, entity ) ); return true; } + else { + // check if the PC contains a deleted entry, if so return true + final var holder = persistenceContext.getEntityHolder( entityKey ); + final var entry = holder == null ? null : holder.getEntityEntry(); + if ( entry != null && entry.getStatus().isDeletedOrGone() ) { + results.add( i, null ); + return true; + } + } } return false; diff --git a/hibernate-core/src/main/java/org/hibernate/loader/internal/CacheLoadHelper.java b/hibernate-core/src/main/java/org/hibernate/loader/internal/CacheLoadHelper.java index ee20c662bf83..92148ac68eb4 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/internal/CacheLoadHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/internal/CacheLoadHelper.java @@ -198,6 +198,9 @@ private static Object processCachedEntry( instanceToLoad, entityKey ); + if ( entity == null ) { + return null; + } if ( !persister.isInstance( entity ) ) { // Clean up the inconsistent return class entity from the persistence context final var persistenceContext = source.getPersistenceContext(); @@ -266,10 +269,12 @@ private static Object convertCacheEntryToEntity( if ( instanceToLoad != null ) { entity = instanceToLoad; } + else if ( oldHolder != null && oldHolder.getEntity() != null ) { + return oldHolder.getEntityEntry() != null && oldHolder.getEntityEntry().getStatus().isDeletedOrGone() ? null + : oldHolder.getEntity(); + } else { - entity = oldHolder != null && oldHolder.getEntity() != null - ? oldHolder.getEntity() - : source.instantiate( subclassPersister, entityId ); + entity = source.instantiate( subclassPersister, entityId ); } if ( isPersistentAttributeInterceptable( entity ) ) { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/cache/FindMultipleCacheTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/cache/FindMultipleCacheTest.java new file mode 100644 index 000000000000..f13f1ed3c4eb --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/cache/FindMultipleCacheTest.java @@ -0,0 +1,275 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.cache; + +import java.util.List; + +import org.hibernate.FindMultipleOption; +import org.hibernate.annotations.Cache; +import org.hibernate.annotations.CacheConcurrencyStrategy; +import org.hibernate.cfg.AvailableSettings; + +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.JiraKey; +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.AfterEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Basic; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; + +import static org.assertj.core.api.Assertions.assertThat; + +@DomainModel( + annotatedClasses = { + FindMultipleCacheTest.CachedEntity.class, + FindMultipleCacheTest.NotCachedEntity.class + } +) +@ServiceRegistry( + settings = { + @Setting(name = AvailableSettings.USE_SECOND_LEVEL_CACHE, value = "true"), + @Setting(name = AvailableSettings.USE_QUERY_CACHE, value = "true"), + @Setting(name = AvailableSettings.SHOW_SQL, value = "true"), + @Setting(name = AvailableSettings.FORMAT_SQL, value = "true"), + } +) +@SessionFactory(generateStatistics = true) +@JiraKey("HHH-20515") +public class FindMultipleCacheTest { + + @AfterEach + public void tearDown(SessionFactoryScope scope) { + scope.getSessionFactory().getSchemaManager().truncate(); + scope.getSessionFactory().getCache().evictAllRegions(); + } + + @Test + public void testFindMultipleWithDisabledPreservesModifiedStateWith2LCPresent(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.persist( new CachedEntity( 1L, "originalName" ) ); + } ); + + scope.inTransaction( session -> { + session.find( CachedEntity.class, 1L ); + } ); + + scope.inTransaction( session -> { + CachedEntity entity = session.find( CachedEntity.class, 1L ); + entity.setName( "modifiedName" ); + + List results = session.findMultiple( + CachedEntity.class, + List.of( 1L ), + FindMultipleOption.SessionCheckMode.DISABLED + ); + + assertThat( results ).hasSize( 1 ); + assertThat( results.get( 0 ) ).isSameAs( entity ); + assertThat( results.get( 0 ).getName() ).isEqualTo( "modifiedName" ); + } ); + } + + @Test + public void testFindMultipleWithDisabledAndUninitializedProxyInPersistenceContext( + SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.persist( new CachedEntity( 1L, "name1" ) ); + } ); + + scope.inTransaction( session -> { + session.find( CachedEntity.class, 1L ); + } ); + + scope.inTransaction( session -> { + CachedEntity proxy = + session.getReference( CachedEntity.class, 1L ); + + List results = session.findMultiple( + CachedEntity.class, + List.of( 1L ), + FindMultipleOption.SessionCheckMode.DISABLED + + ); + + assertThat( results ).hasSize( 1 ); + assertThat( results.get( 0 ) ).isSameAs( proxy ); + } ); + } + + @Test + public void testFindMultipleWithEnabledPreservesModifiedStateWith2LCPresent(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.persist( new CachedEntity( 1L, "originalName" ) ); + } ); + + scope.inTransaction( session -> { + session.find( CachedEntity.class, 1L ); + } ); + + scope.inTransaction( session -> { + CachedEntity entity = session.find( CachedEntity.class, 1L ); + entity.setName( "modifiedName" ); + + List results = session.findMultiple( + CachedEntity.class, + List.of( 1L ), + FindMultipleOption.SessionCheckMode.ENABLED + ); + + assertThat( results ).hasSize( 1 ); + assertThat( results.get( 0 ) ).isSameAs( entity ); + assertThat( results.get( 0 ).getName() ).isEqualTo( "modifiedName" ); + } ); + } + + @Test + public void testFindMultiplePreservesModifiedStateForNonCachedEntity(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.persist( new NotCachedEntity( 1L, "originalName" ) ); + } ); + + scope.inTransaction( session -> { + NotCachedEntity entity = session.findMultiple( NotCachedEntity.class, List.of( 1L ) ).get( 0 ); + assertThat( entity.getName() ).isEqualTo( "originalName" ); + entity.setName( "modifiedName" ); + + NotCachedEntity entity2 = session.findMultiple( NotCachedEntity.class, List.of( 1L ) ).get( 0 ); + assertThat( entity2 ).isSameAs( entity ); + assertThat( entity2.getName() ).isEqualTo( "modifiedName" ); + } ); + } + + @Test + public void testFindMultipleRetrievesPersistedChangesFromCache(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.persist( new CachedEntity( 1L, "originalName" ) ); + } ); + + scope.inTransaction( session -> { + CachedEntity entity = session.findMultiple( CachedEntity.class, List.of( 1L ) ).get( 0 ); + entity.setName( "persistedName" ); + } ); + + scope.inTransaction( session -> { + CachedEntity entity = session.findMultiple( CachedEntity.class, List.of( 1L ) ).get( 0 ); + assertThat( entity.getName() ).isEqualTo( "persistedName" ); + } ); + } + + @Test + public void testFindMultipleWithDisabledPreservesModifiedStateFromDb(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.persist( new CachedEntity( 1L, "originalName" ) ); + } ); + + scope.inTransaction( session -> { + CachedEntity entity = session.find( CachedEntity.class, 1L ); + + entity.setName( "modifiedName" ); + + List results = session.findMultiple( + CachedEntity.class, + List.of( 1L ), + FindMultipleOption.SessionCheckMode.DISABLED + ); + + assertThat( results ).hasSize( 1 ); + assertThat( results.get( 0 ) ).isSameAs( entity ); + assertThat( results.get( 0 ).getName() ).isEqualTo( "modifiedName" ); + } ); + } + + @Test + public void testFindMultipleWithDisabledReturnsDeletedEntity(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.persist( new CachedEntity( 1L, "name1" ) ); + session.persist( new CachedEntity( 2L, "name2" ) ); + } ); + + scope.inTransaction( session -> { + CachedEntity deletedEntity = + session.find( CachedEntity.class, 1L ); + + session.remove( deletedEntity ); + + List results = session.findMultiple( + CachedEntity.class, + List.of( 1L, 2L ), + FindMultipleOption.SessionCheckMode.DISABLED + ); + + CachedEntity returnedEntity = results.get( 0 ); + assertThat( results ).hasSize( 2 ); + + assertThat( results.get( 0 ) ).isNull(); + assertThat( results.get( 1 ) ).isNotNull(); + assertThat( results.get( 1 ).getName() ).isEqualTo( "name2" ); + } ); + } + + @Entity(name = "CachedEntity") + @Cache(usage = CacheConcurrencyStrategy.READ_WRITE) + public static class CachedEntity { + @Id + private Long id; + + @Basic + private String name; + + public CachedEntity() { + } + + public CachedEntity(Long id, String name) { + this.id = id; + this.name = name; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + @Entity(name = "NotCachedEntity") + public static class NotCachedEntity { + @Id + private Long id; + + @Basic + private String name; + + public NotCachedEntity() { + } + + public NotCachedEntity(Long id, String name) { + this.id = id; + this.name = name; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } +}