diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java index 0b2f8e0ea9c2..1327fdc145a2 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java @@ -22,6 +22,7 @@ import org.hibernate.annotations.NotFoundAction; import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer; import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementAsProxyLazinessInterceptor; +import org.hibernate.cache.spi.access.AccessType; import org.hibernate.cache.spi.access.EntityDataAccess; import org.hibernate.cache.spi.entry.CacheEntry; import org.hibernate.engine.spi.EntityEntry; @@ -1389,16 +1390,26 @@ private void putInCache( // we need to be careful not to clobber the lock here in the cache so that it can be rolled back if need be final EventManager eventManager = session.getEventManager(); if ( persistenceContext.wasInsertedDuringTransaction( data.concreteDescriptor, data.entityKey.getIdentifier() ) ) { - boolean update = false; + boolean cacheContentChanged = false; final HibernateMonitoringEvent cachePutEvent = eventManager.beginCachePutEvent(); try { - update = cacheAccess.update( - session, - cacheKey, - data.concreteDescriptor.getCacheEntryStructure().structure( cacheEntry ), - version, - version - ); + // Updating the cache entry for entities that were inserted in this transaction + // only makes sense for transactional caches. Other implementations no-op for #update + // Since #afterInsert will run at the end of the transaction, + // the state of an entity will be stored in the cache eventually. + // Refreshing an inserted entity is a potential concern, + // because one might think that we are missing to store the refreshed data in the cache. + // Actually an entity is evicted from the cache on refresh for non-transactional caches + // via CachedDomainDataAccess#unlockItem after transaction completion, so all is fine. + if ( cacheAccess.getAccessType() == AccessType.TRANSACTIONAL ) { + cacheContentChanged = cacheAccess.update( + session, + cacheKey, + data.concreteDescriptor.getCacheEntryStructure().structure( cacheEntry ), + version, + version + ); + } } finally { eventManager.completeCachePutEvent( @@ -1406,7 +1417,7 @@ private void putInCache( session, cacheAccess, data.concreteDescriptor, - update, + cacheContentChanged, EventManager.CacheActionDescription.ENTITY_UPDATE ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/caching/ChacheReadOnlyStartegyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/CacheReadOnlyStrategyTest.java similarity index 78% rename from hibernate-core/src/test/java/org/hibernate/orm/test/caching/ChacheReadOnlyStartegyTest.java rename to hibernate-core/src/test/java/org/hibernate/orm/test/caching/CacheReadOnlyStrategyTest.java index b0b3e11a221c..b09720f6a66b 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/caching/ChacheReadOnlyStartegyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/CacheReadOnlyStrategyTest.java @@ -18,20 +18,20 @@ import jakarta.persistence.Id; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; -import static org.junit.Assert.assertFalse; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @JiraKey("HHH-17997") @Jpa( annotatedClasses = { - ChacheReadOnlyStartegyTest.TestEntity.class + CacheReadOnlyStrategyTest.TestEntity.class }, integrationSettings = { @Setting(name = AvailableSettings.USE_SECOND_LEVEL_CACHE, value = "true"), @Setting(name = AvailableSettings.USE_QUERY_CACHE, value = "false"), } ) -public class ChacheReadOnlyStartegyTest { +public class CacheReadOnlyStrategyTest { @AfterEach public void tearDown(EntityManagerFactoryScope scope) { @@ -43,7 +43,7 @@ public void tearDown(EntityManagerFactoryScope scope) { @Test public void testPersistThenClearAndQuery(EntityManagerFactoryScope scope) { - final long testEntityId = 1l; + final long testEntityId = 1L; scope.inTransaction( entityManager -> { @@ -71,26 +71,21 @@ public void testPersistThenClearAndQuery(EntityManagerFactoryScope scope) { @Test public void testPersistThenClearAndQueryWithRollback(EntityManagerFactoryScope scope) { - final long testEntityId = 1l; + final long testEntityId = 1L; - scope.inEntityManager( + scope.inTransaction( entityManager -> { - entityManager.getTransaction().begin(); - try { - TestEntity entity = new TestEntity( testEntityId, "test" ); - entityManager.persist( entity ); - entityManager.flush(); - entityManager.clear(); - List results = entityManager.createQuery( - "select t from TestEntity t where t.id = :id", - TestEntity.class - ).setParameter( "id", testEntityId ).getResultList(); - - assertThat( results.size() ).isEqualTo( 1 ); - } - finally { - entityManager.getTransaction().rollback(); - } + TestEntity entity = new TestEntity( testEntityId, "test" ); + entityManager.persist( entity ); + entityManager.flush(); + entityManager.clear(); + List results = entityManager.createQuery( + "select t from TestEntity t where t.id = :id", + TestEntity.class + ).setParameter( "id", testEntityId ).getResultList(); + + entityManager.getTransaction().setRollbackOnly(); + assertThat( results.size() ).isEqualTo( 1 ); } ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/caching/CachingWithTriggerTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/CachingWithTriggerTest.java new file mode 100644 index 000000000000..04ef656f0aab --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/CachingWithTriggerTest.java @@ -0,0 +1,171 @@ +package org.hibernate.orm.test.caching; + +import java.time.Instant; +import java.util.List; + +import org.hibernate.annotations.Cache; +import org.hibernate.annotations.CacheConcurrencyStrategy; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.dialect.PostgreSQLDialect; + +import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.JiraKey; +import org.hibernate.testing.orm.junit.Jpa; +import org.hibernate.testing.orm.junit.RequiresDialect; +import org.hibernate.testing.orm.junit.Setting; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Cacheable; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@JiraKey("HHH-17997") +@Jpa( + annotatedClasses = { + CachingWithTriggerTest.TestEntity.class + }, + integrationSettings = { + @Setting(name = AvailableSettings.USE_SECOND_LEVEL_CACHE, value = "true"), + @Setting(name = AvailableSettings.USE_QUERY_CACHE, value = "false"), + } +) +@RequiresDialect(value = PostgreSQLDialect.class, comment = "To write a trigger only once") +public class CachingWithTriggerTest { + + private static final String TRIGGER = "begin NEW.lastUpdatedAt = current_timestamp; return NEW; end;"; + + @BeforeEach + public void prepare(EntityManagerFactoryScope scope) { + scope.inTransaction( + s -> { + s.createNativeQuery( "create function update_ts_func() returns trigger language plpgsql as $$ " + TRIGGER + " $$" ) + .executeUpdate(); + s.createNativeQuery( "create trigger update_ts before insert on TestEntity for each row execute procedure update_ts_func()" ) + .executeUpdate(); + } + ); + } + + @AfterEach + public void cleanup(EntityManagerFactoryScope scope) { + scope.inTransaction( + s -> { + s.createNativeQuery( "drop trigger if exists update_ts on TestEntity" ) + .executeUpdate(); + s.createNativeQuery( "drop function if exists update_ts_func()" ) + .executeUpdate(); + s.createQuery( "delete from TestEntity" ).executeUpdate(); + } + ); + } + + @Test + public void testPersistThenRefresh(EntityManagerFactoryScope scope) { + final long testEntityId = 1L; + + scope.inTransaction( + entityManager -> { + TestEntity entity = new TestEntity( testEntityId, "test" ); + entityManager.persist( entity ); + entityManager.flush(); + // No reload happens + assertThat( entity.lastUpdatedAt ).isNull(); + } + ); + scope.inTransaction( + entityManager -> { + TestEntity entity = entityManager.find( TestEntity.class, testEntityId ); + entityManager.refresh( entity ); + // On refresh, we want the actual data from the database + assertThat( entity.lastUpdatedAt ).isNotNull(); + } + ); + scope.inTransaction( + entityManager -> { + TestEntity entity = entityManager.find( TestEntity.class, testEntityId ); + // Ensure that we don't get stale data + assertThat( entity.lastUpdatedAt ).isNotNull(); + } + ); + } + + @Test + public void testPersistThenRefreshInTransaction(EntityManagerFactoryScope scope) { + final long testEntityId = 1L; + + scope.inTransaction( + entityManager -> { + TestEntity entity = new TestEntity( testEntityId, "test" ); + entityManager.persist( entity ); + entityManager.flush(); + entityManager.refresh( entity ); + // On refresh, we want the actual data from the database + assertThat( entity.lastUpdatedAt ).isNotNull(); + } + ); + + scope.inTransaction( + entityManager -> { + TestEntity entity = entityManager.find( TestEntity.class, testEntityId ); + // Ensure that we don't get stale data + assertThat( entity.lastUpdatedAt ).isNotNull(); + } + ); + } + + @Test + public void testPersistThenRefreshClearAndQueryInTransaction(EntityManagerFactoryScope scope) { + final long testEntityId = 1L; + + scope.inTransaction( + entityManager -> { + TestEntity entity = new TestEntity( testEntityId, "test" ); + entityManager.persist( entity ); + entityManager.flush(); + entityManager.refresh( entity ); + // On refresh, we want the actual data from the database + assertThat( entity.lastUpdatedAt ).isNotNull(); + entityManager.clear(); + + entity = entityManager.find( TestEntity.class, testEntityId ); + // Ensure that we don't get stale data + assertThat( entity.lastUpdatedAt ).isNotNull(); + } + ); + + scope.inTransaction( + entityManager -> { + TestEntity entity = entityManager.find( TestEntity.class, testEntityId ); + // Ensure that we don't get stale data + assertThat( entity.lastUpdatedAt ).isNotNull(); + } + ); + } + + @Entity(name = "TestEntity") + @Cacheable + @Cache(usage = CacheConcurrencyStrategy.READ_ONLY) + public static class TestEntity { + @Id + private Long id; + + private String name; + + private Instant lastUpdatedAt; + + public TestEntity() { + } + + public TestEntity(Long id, String name) { + this.id = id; + this.name = name; + } + } + +}