Skip to content

Commit 54c1c4a

Browse files
dreab8beikov
dreab8
authored and
beikov
committed
HHH-17997 Querying an Entity with CacheConcurrencyStrategy.READONLY throws UnsupportedOperationException: Can't update readonly object
1 parent 20acd52 commit 54c1c4a

File tree

3 files changed

+208
-31
lines changed

3 files changed

+208
-31
lines changed

‎hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.hibernate.annotations.NotFoundAction;
2323
import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer;
2424
import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementAsProxyLazinessInterceptor;
25+
import org.hibernate.cache.spi.access.AccessType;
2526
import org.hibernate.cache.spi.access.EntityDataAccess;
2627
import org.hibernate.cache.spi.entry.CacheEntry;
2728
import org.hibernate.engine.spi.EntityEntry;
@@ -1389,24 +1390,34 @@ private void putInCache(
13891390
// we need to be careful not to clobber the lock here in the cache so that it can be rolled back if need be
13901391
final EventManager eventManager = session.getEventManager();
13911392
if ( persistenceContext.wasInsertedDuringTransaction( data.concreteDescriptor, data.entityKey.getIdentifier() ) ) {
1392-
boolean update = false;
1393+
boolean cacheContentChanged = false;
13931394
final HibernateMonitoringEvent cachePutEvent = eventManager.beginCachePutEvent();
13941395
try {
1395-
update = cacheAccess.update(
1396-
session,
1397-
cacheKey,
1398-
data.concreteDescriptor.getCacheEntryStructure().structure( cacheEntry ),
1399-
version,
1400-
version
1401-
);
1396+
// Updating the cache entry for entities that were inserted in this transaction
1397+
// only makes sense for transactional caches. Other implementations no-op for #update
1398+
// Since #afterInsert will run at the end of the transaction,
1399+
// the state of an entity will be stored in the cache eventually.
1400+
// Refreshing an inserted entity is a potential concern,
1401+
// because one might think that we are missing to store the refreshed data in the cache.
1402+
// Actually an entity is evicted from the cache on refresh for non-transactional caches
1403+
// via CachedDomainDataAccess#unlockItem after transaction completion, so all is fine.
1404+
if ( cacheAccess.getAccessType() == AccessType.TRANSACTIONAL ) {
1405+
cacheContentChanged = cacheAccess.update(
1406+
session,
1407+
cacheKey,
1408+
data.concreteDescriptor.getCacheEntryStructure().structure( cacheEntry ),
1409+
version,
1410+
version
1411+
);
1412+
}
14021413
}
14031414
finally {
14041415
eventManager.completeCachePutEvent(
14051416
cachePutEvent,
14061417
session,
14071418
cacheAccess,
14081419
data.concreteDescriptor,
1409-
update,
1420+
cacheContentChanged,
14101421
EventManager.CacheActionDescription.ENTITY_UPDATE
14111422
);
14121423
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/caching/ChacheReadOnlyStartegyTest.java renamed to ‎hibernate-core/src/test/java/org/hibernate/orm/test/caching/CacheReadOnlyStrategyTest.java

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,20 @@
1818
import jakarta.persistence.Id;
1919

2020
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
21-
import static org.junit.Assert.assertFalse;
21+
import static org.junit.jupiter.api.Assertions.assertFalse;
2222
import static org.junit.jupiter.api.Assertions.assertTrue;
2323

2424
@JiraKey("HHH-17997")
2525
@Jpa(
2626
annotatedClasses = {
27-
ChacheReadOnlyStartegyTest.TestEntity.class
27+
CacheReadOnlyStrategyTest.TestEntity.class
2828
},
2929
integrationSettings = {
3030
@Setting(name = AvailableSettings.USE_SECOND_LEVEL_CACHE, value = "true"),
3131
@Setting(name = AvailableSettings.USE_QUERY_CACHE, value = "false"),
3232
}
3333
)
34-
public class ChacheReadOnlyStartegyTest {
34+
public class CacheReadOnlyStrategyTest {
3535

3636
@AfterEach
3737
public void tearDown(EntityManagerFactoryScope scope) {
@@ -43,7 +43,7 @@ public void tearDown(EntityManagerFactoryScope scope) {
4343

4444
@Test
4545
public void testPersistThenClearAndQuery(EntityManagerFactoryScope scope) {
46-
final long testEntityId = 1l;
46+
final long testEntityId = 1L;
4747

4848
scope.inTransaction(
4949
entityManager -> {
@@ -71,26 +71,21 @@ public void testPersistThenClearAndQuery(EntityManagerFactoryScope scope) {
7171

7272
@Test
7373
public void testPersistThenClearAndQueryWithRollback(EntityManagerFactoryScope scope) {
74-
final long testEntityId = 1l;
74+
final long testEntityId = 1L;
7575

76-
scope.inEntityManager(
76+
scope.inTransaction(
7777
entityManager -> {
78-
entityManager.getTransaction().begin();
79-
try {
80-
TestEntity entity = new TestEntity( testEntityId, "test" );
81-
entityManager.persist( entity );
82-
entityManager.flush();
83-
entityManager.clear();
84-
List<TestEntity> results = entityManager.createQuery(
85-
"select t from TestEntity t where t.id = :id",
86-
TestEntity.class
87-
).setParameter( "id", testEntityId ).getResultList();
88-
89-
assertThat( results.size() ).isEqualTo( 1 );
90-
}
91-
finally {
92-
entityManager.getTransaction().rollback();
93-
}
78+
TestEntity entity = new TestEntity( testEntityId, "test" );
79+
entityManager.persist( entity );
80+
entityManager.flush();
81+
entityManager.clear();
82+
List<TestEntity> results = entityManager.createQuery(
83+
"select t from TestEntity t where t.id = :id",
84+
TestEntity.class
85+
).setParameter( "id", testEntityId ).getResultList();
86+
87+
entityManager.getTransaction().setRollbackOnly();
88+
assertThat( results.size() ).isEqualTo( 1 );
9489
}
9590
);
9691

‎hibernate-core/src/test/java/org/hibernate/orm/test/caching/CachingWithTriggerTest.java

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
package org.hibernate.orm.test.caching;
2+
3+
import java.time.Instant;
4+
import java.util.List;
5+
6+
import org.hibernate.annotations.Cache;
7+
import org.hibernate.annotations.CacheConcurrencyStrategy;
8+
import org.hibernate.cfg.AvailableSettings;
9+
import org.hibernate.dialect.PostgreSQLDialect;
10+
11+
import org.hibernate.testing.orm.junit.EntityManagerFactoryScope;
12+
import org.hibernate.testing.orm.junit.JiraKey;
13+
import org.hibernate.testing.orm.junit.Jpa;
14+
import org.hibernate.testing.orm.junit.RequiresDialect;
15+
import org.hibernate.testing.orm.junit.Setting;
16+
import org.junit.jupiter.api.AfterEach;
17+
import org.junit.jupiter.api.BeforeEach;
18+
import org.junit.jupiter.api.Test;
19+
20+
import jakarta.persistence.Cacheable;
21+
import jakarta.persistence.Entity;
22+
import jakarta.persistence.Id;
23+
24+
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
25+
import static org.junit.jupiter.api.Assertions.assertFalse;
26+
import static org.junit.jupiter.api.Assertions.assertTrue;
27+
28+
@JiraKey("HHH-17997")
29+
@Jpa(
30+
annotatedClasses = {
31+
CachingWithTriggerTest.TestEntity.class
32+
},
33+
integrationSettings = {
34+
@Setting(name = AvailableSettings.USE_SECOND_LEVEL_CACHE, value = "true"),
35+
@Setting(name = AvailableSettings.USE_QUERY_CACHE, value = "false"),
36+
}
37+
)
38+
@RequiresDialect(value = PostgreSQLDialect.class, comment = "To write a trigger only once")
39+
public class CachingWithTriggerTest {
40+
41+
private static final String TRIGGER = "begin NEW.lastUpdatedAt = current_timestamp; return NEW; end;";
42+
43+
@BeforeEach
44+
public void prepare(EntityManagerFactoryScope scope) {
45+
scope.inTransaction(
46+
s -> {
47+
s.createNativeQuery( "create function update_ts_func() returns trigger language plpgsql as $$ " + TRIGGER + " $$" )
48+
.executeUpdate();
49+
s.createNativeQuery( "create trigger update_ts before insert on TestEntity for each row execute procedure update_ts_func()" )
50+
.executeUpdate();
51+
}
52+
);
53+
}
54+
55+
@AfterEach
56+
public void cleanup(EntityManagerFactoryScope scope) {
57+
scope.inTransaction(
58+
s -> {
59+
s.createNativeQuery( "drop trigger if exists update_ts on TestEntity" )
60+
.executeUpdate();
61+
s.createNativeQuery( "drop function if exists update_ts_func()" )
62+
.executeUpdate();
63+
s.createQuery( "delete from TestEntity" ).executeUpdate();
64+
}
65+
);
66+
}
67+
68+
@Test
69+
public void testPersistThenRefresh(EntityManagerFactoryScope scope) {
70+
final long testEntityId = 1L;
71+
72+
scope.inTransaction(
73+
entityManager -> {
74+
TestEntity entity = new TestEntity( testEntityId, "test" );
75+
entityManager.persist( entity );
76+
entityManager.flush();
77+
// No reload happens
78+
assertThat( entity.lastUpdatedAt ).isNull();
79+
}
80+
);
81+
scope.inTransaction(
82+
entityManager -> {
83+
TestEntity entity = entityManager.find( TestEntity.class, testEntityId );
84+
entityManager.refresh( entity );
85+
// On refresh, we want the actual data from the database
86+
assertThat( entity.lastUpdatedAt ).isNotNull();
87+
}
88+
);
89+
scope.inTransaction(
90+
entityManager -> {
91+
TestEntity entity = entityManager.find( TestEntity.class, testEntityId );
92+
// Ensure that we don't get stale data
93+
assertThat( entity.lastUpdatedAt ).isNotNull();
94+
}
95+
);
96+
}
97+
98+
@Test
99+
public void testPersistThenRefreshInTransaction(EntityManagerFactoryScope scope) {
100+
final long testEntityId = 1L;
101+
102+
scope.inTransaction(
103+
entityManager -> {
104+
TestEntity entity = new TestEntity( testEntityId, "test" );
105+
entityManager.persist( entity );
106+
entityManager.flush();
107+
entityManager.refresh( entity );
108+
// On refresh, we want the actual data from the database
109+
assertThat( entity.lastUpdatedAt ).isNotNull();
110+
}
111+
);
112+
113+
scope.inTransaction(
114+
entityManager -> {
115+
TestEntity entity = entityManager.find( TestEntity.class, testEntityId );
116+
// Ensure that we don't get stale data
117+
assertThat( entity.lastUpdatedAt ).isNotNull();
118+
}
119+
);
120+
}
121+
122+
@Test
123+
public void testPersistThenRefreshClearAndQueryInTransaction(EntityManagerFactoryScope scope) {
124+
final long testEntityId = 1L;
125+
126+
scope.inTransaction(
127+
entityManager -> {
128+
TestEntity entity = new TestEntity( testEntityId, "test" );
129+
entityManager.persist( entity );
130+
entityManager.flush();
131+
entityManager.refresh( entity );
132+
// On refresh, we want the actual data from the database
133+
assertThat( entity.lastUpdatedAt ).isNotNull();
134+
entityManager.clear();
135+
136+
entity = entityManager.find( TestEntity.class, testEntityId );
137+
// Ensure that we don't get stale data
138+
assertThat( entity.lastUpdatedAt ).isNotNull();
139+
}
140+
);
141+
142+
scope.inTransaction(
143+
entityManager -> {
144+
TestEntity entity = entityManager.find( TestEntity.class, testEntityId );
145+
// Ensure that we don't get stale data
146+
assertThat( entity.lastUpdatedAt ).isNotNull();
147+
}
148+
);
149+
}
150+
151+
@Entity(name = "TestEntity")
152+
@Cacheable
153+
@Cache(usage = CacheConcurrencyStrategy.READ_ONLY)
154+
public static class TestEntity {
155+
@Id
156+
private Long id;
157+
158+
private String name;
159+
160+
private Instant lastUpdatedAt;
161+
162+
public TestEntity() {
163+
}
164+
165+
public TestEntity(Long id, String name) {
166+
this.id = id;
167+
this.name = name;
168+
}
169+
}
170+
171+
}

0 commit comments

Comments
 (0)