Skip to content

Commit 86a274d

Browse files
gavinkinggavinking
committed
change handling of natural id resolutions in EntityUpdateAction
it's conceptually wrong to do work in the constructor of an Action instead of in its execute() method
1 parent 5bd506a commit 86a274d

File tree

6 files changed

+92
-59
lines changed

6 files changed

+92
-59
lines changed

‎hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import org.hibernate.AssertionFailure;
88
import org.hibernate.HibernateException;
99
import org.hibernate.cache.spi.access.SoftLock;
10+
import org.hibernate.engine.spi.PersistenceContext;
1011
import org.hibernate.engine.spi.SharedSessionContractImplementor;
1112
import org.hibernate.event.spi.EventSource;
1213
import org.hibernate.event.spi.PostCommitDeleteEventListener;
@@ -106,7 +107,7 @@ public void execute() throws HibernateException {
106107

107108
final boolean veto = isInstanceLoaded() && preDelete();
108109

109-
handleNaturalIdResolutions( persister, session );
110+
handleNaturalIdLocalResolutions( persister, session.getPersistenceContextInternal() );
110111

111112
final Object ck = lockCacheItem();
112113

@@ -137,10 +138,10 @@ public void execute() throws HibernateException {
137138
}
138139
}
139140

140-
private void handleNaturalIdResolutions(EntityPersister persister, EventSource session) {
141+
private void handleNaturalIdLocalResolutions(EntityPersister persister, PersistenceContext context) {
141142
final var naturalIdMapping = persister.getNaturalIdMapping();
142143
if ( naturalIdMapping != null ) {
143-
naturalIdValues = session.getPersistenceContextInternal().getNaturalIdResolutions()
144+
naturalIdValues = context.getNaturalIdResolutions()
144145
.removeLocalResolution(
145146
getId(),
146147
naturalIdMapping.extractNaturalIdFromEntityState( state ),

‎hibernate-core/src/main/java/org/hibernate/action/internal/EntityInsertAction.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,7 @@ protected void putCacheIfNecessary() {
163163
final var session = getSession();
164164
if ( isCachePutEnabled( persister, session ) ) {
165165
final var factory = session.getFactory();
166-
final var cacheEntry = persister.buildCacheEntry( getInstance(), getState(), version, session );
167-
this.cacheEntry = persister.getCacheEntryStructure().structure( cacheEntry );
166+
cacheEntry = buildStructuredCacheEntry();
168167
final var cache = persister.getCacheAccessStrategy();
169168
final Object ck = cache.generateCacheKey( getId(), persister, factory, session.getTenantIdentifier() );
170169
final boolean put = cacheInsert( persister, ck );
@@ -179,6 +178,12 @@ protected void putCacheIfNecessary() {
179178
}
180179
}
181180

181+
private Object buildStructuredCacheEntry() {
182+
final var persister = getPersister();
183+
final var cacheEntry = persister.buildCacheEntry( getInstance(), getState(), version, getSession() );
184+
return persister.getCacheEntryStructure().structure( cacheEntry );
185+
}
186+
182187
protected boolean cacheInsert(EntityPersister persister, Object ck) {
183188
final var session = getSession();
184189
final var eventMonitor = session.getEventMonitor();

‎hibernate-core/src/main/java/org/hibernate/action/internal/EntityUpdateAction.java‎

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ public class EntityUpdateAction extends EntityAction {
4141
private final boolean hasDirtyCollection;
4242
private final Object rowId;
4343

44-
private final NaturalIdMapping naturalIdMapping;
4544
private final Object previousNaturalIdValues;
4645

4746
private Object nextVersion;
@@ -83,22 +82,22 @@ public EntityUpdateAction(
8382
this.hasDirtyCollection = hasDirtyCollection;
8483
this.rowId = rowId;
8584

86-
naturalIdMapping = persister.getNaturalIdMapping();
87-
if ( naturalIdMapping == null ) {
88-
previousNaturalIdValues = null;
89-
}
90-
else {
91-
previousNaturalIdValues =
92-
previousState == null
93-
? session.getPersistenceContextInternal().getNaturalIdSnapshot( id, persister )
94-
: naturalIdMapping.extractNaturalIdFromEntityState( previousState );
95-
session.getPersistenceContextInternal().getNaturalIdResolutions().manageLocalResolution(
96-
id,
97-
naturalIdMapping.extractNaturalIdFromEntityState( state ),
98-
persister,
99-
CachedNaturalIdValueSource.UPDATE
100-
);
101-
}
85+
final var naturalIdMapping = persister.getNaturalIdMapping();
86+
this.previousNaturalIdValues =
87+
naturalIdMapping == null
88+
? null
89+
: determinePreviousNaturalIdValues( persister, naturalIdMapping, id, previousState, session );
90+
}
91+
92+
private static Object determinePreviousNaturalIdValues(
93+
EntityPersister persister,
94+
NaturalIdMapping naturalIdMapping,
95+
Object id,
96+
Object[] previousState,
97+
SharedSessionContractImplementor session) {
98+
return previousState == null
99+
? session.getPersistenceContextInternal().getNaturalIdSnapshot( id, persister )
100+
: naturalIdMapping.extractNaturalIdFromEntityState( previousState );
102101
}
103102

104103
protected Object[] getState() {
@@ -121,7 +120,7 @@ protected boolean hasDirtyCollection() {
121120
}
122121

123122
protected NaturalIdMapping getNaturalIdMapping() {
124-
return naturalIdMapping;
123+
return getPersister().getNaturalIdMapping();
125124
}
126125

127126
protected Object getPreviousNaturalIdValues() {
@@ -145,8 +144,12 @@ public void execute() throws HibernateException {
145144
if ( !preUpdate() ) {
146145
final var persister = getPersister();
147146
final var session = getSession();
147+
final var persistenceContext = session.getPersistenceContextInternal();
148148
final Object id = getId();
149149
final Object instance = getInstance();
150+
151+
handleNaturalIdLocalResolutions( id, persister, persistenceContext );
152+
150153
final Object previousVersion = getPreviousVersion();
151154
final Object ck = lockCacheItem( previousVersion );
152155
final var eventMonitor = session.getEventMonitor();
@@ -170,15 +173,14 @@ public void execute() throws HibernateException {
170173
finally {
171174
eventMonitor.completeEntityUpdateEvent( event, id, persister.getEntityName(), success, session );
172175
}
173-
final var persistenceContext = session.getPersistenceContextInternal();
174-
final var entry = persistenceContext.getEntry( instance );
176+
final var entry = session.getPersistenceContextInternal().getEntry( instance );
175177
if ( entry == null ) {
176178
throw new AssertionFailure( "possible non thread safe access to session" );
177179
}
178180
handleGeneratedProperties( entry, generatedValues );
179181
handleDeleted( entry );
180182
updateCacheItem( previousVersion, ck, entry );
181-
handleNaturalIdResolutions( persister, persistenceContext, id );
183+
handleNaturalIdSharedResolutions( id, persister, persistenceContext );
182184
postUpdate();
183185

184186
final var statistics = session.getFactory().getStatistics();
@@ -188,7 +190,20 @@ public void execute() throws HibernateException {
188190
}
189191
}
190192

191-
protected void handleNaturalIdResolutions(EntityPersister persister, PersistenceContext context, Object id) {
193+
private void handleNaturalIdLocalResolutions(Object id, EntityPersister persister, PersistenceContext context) {
194+
final var naturalIdMapping = persister.getNaturalIdMapping();
195+
if ( naturalIdMapping != null) {
196+
context.getNaturalIdResolutions().manageLocalResolution(
197+
id,
198+
naturalIdMapping.extractNaturalIdFromEntityState( state ),
199+
persister,
200+
CachedNaturalIdValueSource.UPDATE
201+
);
202+
}
203+
}
204+
205+
protected void handleNaturalIdSharedResolutions(Object id, EntityPersister persister, PersistenceContext context) {
206+
final var naturalIdMapping = persister.getNaturalIdMapping();
192207
if ( naturalIdMapping != null ) {
193208
context.getNaturalIdResolutions().manageSharedResolution(
194209
id,
@@ -209,8 +224,7 @@ protected void updateCacheItem(Object previousVersion, Object ck, EntityEntry en
209224
}
210225
else if ( session.getCacheMode().isPutEnabled() ) {
211226
//TODO: inefficient if that cache is just going to ignore the updated state!
212-
final var cacheEntry = persister.buildCacheEntry( getInstance(), state, nextVersion, getSession() );
213-
this.cacheEntry = persister.getCacheEntryStructure().structure( cacheEntry );
227+
cacheEntry = buildStructuredCacheEntry();
214228
final boolean put = updateCache( persister, previousVersion, ck );
215229

216230
final var statistics = session.getFactory().getStatistics();
@@ -224,6 +238,12 @@ else if ( session.getCacheMode().isPutEnabled() ) {
224238
}
225239
}
226240

241+
private Object buildStructuredCacheEntry() {
242+
final var persister = getPersister();
243+
final var cacheEntry = persister.buildCacheEntry( getInstance(), state, nextVersion, getSession() );
244+
return persister.getCacheEntryStructure().structure( cacheEntry );
245+
}
246+
227247
private static boolean isCacheInvalidationRequired(
228248
EntityPersister persister,
229249
SharedSessionContractImplementor session) {

‎hibernate-core/src/main/java/org/hibernate/engine/internal/EntityEntryImpl.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,15 +450,16 @@ else if ( !persister.isMutable() ) {
450450
else {
451451
setStatus( MANAGED );
452452
loadedState = persister.getValues( entity );
453+
final var context = getPersistenceContext();
453454
TypeHelper.deepCopy(
454455
loadedState,
455456
persister.getPropertyTypes(),
456457
persister.getPropertyCheckability(),
457458
loadedState,
458-
getPersistenceContext().getSession()
459+
context.getSession()
459460
);
460461
if ( persister.hasNaturalIdentifier() ) {
461-
getPersistenceContext().getNaturalIdResolutions().manageLocalResolution(
462+
context.getNaturalIdResolutions().manageLocalResolution(
462463
id,
463464
persister.getNaturalIdMapping().extractNaturalIdFromEntityState( loadedState ),
464465
persister,

‎hibernate-core/src/main/java/org/hibernate/internal/NaturalIdHelper.java‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,12 @@
44
*/
55
package org.hibernate.internal;
66

7-
import org.hibernate.engine.spi.EntityEntry;
8-
import org.hibernate.engine.spi.EntityKey;
9-
import org.hibernate.engine.spi.PersistenceContext;
107
import org.hibernate.engine.spi.SharedSessionContractImplementor;
118
import org.hibernate.engine.spi.Status;
129
import org.hibernate.id.IdentifierGenerationException;
1310
import org.hibernate.metamodel.mapping.EntityMappingType;
1411
import org.hibernate.persister.entity.EntityPersister;
1512

16-
import java.util.Collection;
17-
1813
import static org.hibernate.engine.internal.NaturalIdLogging.NATURAL_ID_LOGGER;
1914

2015
/**
@@ -50,22 +45,19 @@ public static void performAnyNeededCrossReferenceSynchronizations(
5045
&& entityMappingType.getNaturalIdMapping().isMutable()
5146
// skip synchronization when not in a transaction
5247
&& session.isTransactionInProgress() ) {
53-
final EntityPersister entityPersister = entityMappingType.getEntityPersister();
54-
final PersistenceContext persistenceContext = session.getPersistenceContextInternal();
55-
final Collection<?> cachedResolutions =
56-
persistenceContext.getNaturalIdResolutions()
57-
.getCachedPkResolutions( entityPersister );
48+
final var persister = entityMappingType.getEntityPersister();
49+
final var persistenceContext = session.getPersistenceContextInternal();
50+
final var naturalIdResolutions = persistenceContext.getNaturalIdResolutions();
5851
final boolean loggerDebugEnabled = NATURAL_ID_LOGGER.isDebugEnabled();
59-
for ( Object id : cachedResolutions ) {
60-
final EntityKey entityKey = session.generateEntityKey( id, entityPersister );
52+
for ( Object id : naturalIdResolutions.getCachedPkResolutions( persister ) ) {
53+
final var entityKey = session.generateEntityKey( id, persister );
6154
final Object entity = persistenceContext.getEntity( entityKey );
62-
final EntityEntry entry = persistenceContext.getEntry( entity );
55+
final var entry = persistenceContext.getEntry( entity );
6356
if ( entry != null ) {
6457
if ( entry.requiresDirtyCheck( entity )
6558
// MANAGED is the only status we care about here
6659
&& entry.getStatus() == Status.MANAGED ) {
67-
persistenceContext.getNaturalIdResolutions()
68-
.handleSynchronization( id, entity, entityPersister );
60+
naturalIdResolutions.handleSynchronization( id, entity, persister );
6961
}
7062
}
7163
else {

‎hibernate-core/src/main/java/org/hibernate/internal/OptimisticLockHelper.java‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@
88
import org.hibernate.action.spi.AfterTransactionCompletionProcess;
99
import org.hibernate.cache.spi.access.EntityDataAccess;
1010
import org.hibernate.cache.spi.access.SoftLock;
11-
import org.hibernate.cache.spi.entry.CacheEntry;
1211
import org.hibernate.engine.spi.EntityEntry;
1312
import org.hibernate.engine.spi.SharedSessionContractImplementor;
1413
import org.hibernate.engine.spi.Status;
15-
import org.hibernate.event.monitor.spi.DiagnosticEvent;
1614
import org.hibernate.event.monitor.spi.EventMonitor;
1715
import org.hibernate.persister.entity.EntityPersister;
1816
import org.hibernate.stat.internal.StatsHelper;
@@ -24,12 +22,12 @@ private OptimisticLockHelper() {
2422
}
2523

2624
public static void forceVersionIncrement(Object object, EntityEntry entry, SharedSessionContractImplementor session) {
27-
final EntityPersister persister = entry.getPersister();
25+
final var persister = entry.getPersister();
2826
final Object previousVersion = entry.getVersion();
2927
SoftLock lock = null;
3028
final Object cacheKey;
3129
if ( persister.canWriteToCache() ) {
32-
final EntityDataAccess cache = persister.getCacheAccessStrategy();
30+
final var cache = persister.getCacheAccessStrategy();
3331
cacheKey = cache.generateCacheKey(
3432
entry.getId(),
3533
persister,
@@ -70,8 +68,8 @@ private static Object updateCacheItem(Object entity, Object previousVersion, Obj
7068
}
7169
else if ( session.getCacheMode().isPutEnabled() ) {
7270
//TODO: inefficient if that cache is just going to ignore the updated state!
73-
final CacheEntry ce = persister.buildCacheEntry( entity, entry.getLoadedState(), nextVersion, session );
74-
final Object cacheEntry = persister.getCacheEntryStructure().structure( ce );
71+
final Object cacheEntry =
72+
buildStructuredCacheEntry( entity, nextVersion, entry.getLoadedState(), persister, session );
7573
final boolean put = updateCache( persister, cacheEntry, previousVersion, nextVersion, ck, session );
7674

7775
final var statistics = session.getFactory().getStatistics();
@@ -86,10 +84,26 @@ else if ( session.getCacheMode().isPutEnabled() ) {
8684
return null;
8785
}
8886

89-
private static boolean updateCache(EntityPersister persister, Object cacheEntry, Object previousVersion, Object nextVersion, Object ck, SharedSessionContractImplementor session) {
90-
final EventMonitor eventMonitor = session.getEventMonitor();
91-
final DiagnosticEvent cachePutEvent = eventMonitor.beginCachePutEvent();
92-
final EntityDataAccess cacheAccessStrategy = persister.getCacheAccessStrategy();
87+
private static Object buildStructuredCacheEntry(
88+
Object entity,
89+
Object nextVersion,
90+
Object[] state,
91+
EntityPersister persister,
92+
SharedSessionContractImplementor session) {
93+
final var cacheEntry = persister.buildCacheEntry( entity, state, nextVersion, session );
94+
return persister.getCacheEntryStructure().structure( cacheEntry );
95+
}
96+
97+
private static boolean updateCache(
98+
EntityPersister persister,
99+
Object cacheEntry,
100+
Object previousVersion,
101+
Object nextVersion,
102+
Object ck,
103+
SharedSessionContractImplementor session) {
104+
final var eventMonitor = session.getEventMonitor();
105+
final var cachePutEvent = eventMonitor.beginCachePutEvent();
106+
final var cacheAccessStrategy = persister.getCacheAccessStrategy();
93107
final var eventListenerManager = session.getEventListenerManager();
94108
boolean update = false;
95109
try {
@@ -138,7 +152,7 @@ private CacheCleanupProcess(Object cacheKey, EntityPersister persister, Object p
138152

139153
@Override
140154
public void doAfterTransactionCompletion(boolean success, SharedSessionContractImplementor session) {
141-
final EntityDataAccess cache = persister.getCacheAccessStrategy();
155+
final var cache = persister.getCacheAccessStrategy();
142156
if ( cacheUpdateRequired( success, persister, session ) ) {
143157
cacheAfterUpdate( cache, cacheKey, session );
144158
}
@@ -155,8 +169,8 @@ private static boolean cacheUpdateRequired(boolean success, EntityPersister pers
155169

156170
protected void cacheAfterUpdate(EntityDataAccess cache, Object ck, SharedSessionContractImplementor session) {
157171
final var eventListenerManager = session.getEventListenerManager();
158-
final EventMonitor eventMonitor = session.getEventMonitor();
159-
final DiagnosticEvent cachePutEvent = eventMonitor.beginCachePutEvent();
172+
final var eventMonitor = session.getEventMonitor();
173+
final var cachePutEvent = eventMonitor.beginCachePutEvent();
160174
boolean put = false;
161175
try {
162176
eventListenerManager.cachePutStart();

0 commit comments

Comments
 (0)