Skip to content

Commit 8de4616

Browse files
gavinkinggavinking
committed
clean up code in listener implementations
- try to use smaller methods with fewer params - git rid of early exits - tiny fix for unloaded proxy deletion
1 parent fcd7a45 commit 8de4616

16 files changed

+712
-799
lines changed

‎hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java‎

Lines changed: 71 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ else if ( generatedId == IdentifierGeneratorHelper.POST_INSERT_INDICATOR ) {
130130
persister.getIdentifierGenerator().getClass().getName()
131131
);
132132
}
133-
134133
return performSave( entity, generatedId, persister, false, context, source, true );
135134
}
136135
}
@@ -166,11 +165,28 @@ protected Object performSave(
166165
LOG.tracev( "Saving {0}", MessageHelper.infoString( persister, id, source.getFactory() ) );
167166
}
168167

169-
final EntityKey key;
170-
if ( !useIdentityColumn ) {
171-
key = source.generateEntityKey( id, persister );
168+
final EntityKey key = entityKey( entity, id, persister, useIdentityColumn, source );
169+
if ( invokeSaveLifecycle( entity, persister, source ) ) {
170+
return id;
171+
}
172+
else {
173+
return performSaveOrReplicate(
174+
entity,
175+
key,
176+
persister,
177+
useIdentityColumn,
178+
context,
179+
source,
180+
requiresImmediateIdAccess
181+
);
182+
}
183+
}
184+
185+
private static EntityKey entityKey(Object entity, Object id, EntityPersister persister, boolean useIdentityColumn, EventSource source) {
186+
if ( !useIdentityColumn) {
187+
final EntityKey key = source.generateEntityKey( id, persister );
172188
final PersistenceContext persistenceContext = source.getPersistenceContextInternal();
173-
Object old = persistenceContext.getEntity( key );
189+
final Object old = persistenceContext.getEntity( key );
174190
if ( old != null ) {
175191
if ( persistenceContext.getEntry( old ).getStatus() == Status.DELETED ) {
176192
source.forceFlush( persistenceContext.getEntry( old ) );
@@ -180,24 +196,11 @@ protected Object performSave(
180196
}
181197
}
182198
persister.setIdentifier( entity, id, source );
199+
return key;
183200
}
184201
else {
185-
key = null;
202+
return null;
186203
}
187-
188-
if ( invokeSaveLifecycle( entity, persister, source ) ) {
189-
return id; //EARLY EXIT
190-
}
191-
192-
return performSaveOrReplicate(
193-
entity,
194-
key,
195-
persister,
196-
useIdentityColumn,
197-
context,
198-
source,
199-
requiresImmediateIdAccess
200-
);
201204
}
202205

203206
protected boolean invokeSaveLifecycle(Object entity, EntityPersister persister, EventSource source) {
@@ -238,10 +241,9 @@ protected Object performSaveOrReplicate(
238241
EventSource source,
239242
boolean requiresImmediateIdAccess) {
240243

241-
Object id = key == null ? null : key.getIdentifier();
244+
final Object id = key == null ? null : key.getIdentifier();
242245

243-
boolean inTrx = source.isTransactionInProgress();
244-
boolean shouldDelayIdentityInserts = !inTrx && !requiresImmediateIdAccess;
246+
boolean shouldDelayIdentityInserts = !source.isTransactionInProgress() && !requiresImmediateIdAccess;
245247
final PersistenceContext persistenceContext = source.getPersistenceContextInternal();
246248

247249
// Put a placeholder in entries, so we don't recurse back and try to save() the
@@ -262,29 +264,8 @@ protected Object performSaveOrReplicate(
262264

263265
cascadeBeforeSave( source, persister, entity, context );
264266

265-
Object[] values = persister.getPropertyValuesToInsert( entity, getMergeMap( context ), source );
266-
Type[] types = persister.getPropertyTypes();
267-
268-
boolean substitute = substituteValuesIfNecessary( entity, id, values, persister, source );
269-
270-
if ( persister.hasCollections() ) {
271-
substitute = visitCollectionsBeforeSave( entity, id, values, types, source ) || substitute;
272-
}
273-
274-
if ( substitute ) {
275-
persister.setValues( entity, values );
276-
}
277-
278-
TypeHelper.deepCopy(
279-
values,
280-
types,
281-
persister.getPropertyUpdateability(),
282-
values,
283-
source
284-
);
285-
286267
final AbstractEntityInsertAction insert = addInsertAction(
287-
values,
268+
cloneAndSubstituteValues( entity, persister, context, source, id ),
288269
id,
289270
entity,
290271
persister,
@@ -296,28 +277,60 @@ protected Object performSaveOrReplicate(
296277
// postpone initializing id in case the insert has non-nullable transient dependencies
297278
// that are not resolved until cascadeAfterSave() is executed
298279
cascadeAfterSave( source, persister, entity, context );
299-
if ( useIdentityColumn && insert.isEarlyInsert() ) {
300-
if ( !(insert instanceof EntityIdentityInsertAction) ) {
301-
throw new IllegalStateException(
302-
"Insert should be using an identity column, but action is of unexpected type: " +
303-
insert.getClass().getName()
304-
);
305-
}
306-
id = ((EntityIdentityInsertAction) insert).getGeneratedId();
307280

308-
insert.handleNaturalIdPostSaveNotifications( id );
309-
}
281+
final Object finalId = handleGeneratedId( useIdentityColumn, id, insert );
310282

311283
EntityEntry newEntry = persistenceContext.getEntry( entity );
312-
313284
if ( newEntry != original ) {
314285
EntityEntryExtraState extraState = newEntry.getExtraState( EntityEntryExtraState.class );
315286
if ( extraState == null ) {
316287
newEntry.addExtraState( original.getExtraState( EntityEntryExtraState.class ) );
317288
}
318289
}
319290

320-
return id;
291+
return finalId;
292+
}
293+
294+
private static Object handleGeneratedId(boolean useIdentityColumn, Object id, AbstractEntityInsertAction insert) {
295+
if ( useIdentityColumn && insert.isEarlyInsert() ) {
296+
if ( insert instanceof EntityIdentityInsertAction ) {
297+
Object generatedId = ((EntityIdentityInsertAction) insert).getGeneratedId();
298+
insert.handleNaturalIdPostSaveNotifications( generatedId );
299+
return generatedId;
300+
}
301+
else {
302+
throw new IllegalStateException(
303+
"Insert should be using an identity column, but action is of unexpected type: "
304+
+ insert.getClass().getName()
305+
);
306+
}
307+
}
308+
else {
309+
return id;
310+
}
311+
}
312+
313+
private Object[] cloneAndSubstituteValues(Object entity, EntityPersister persister, C context, EventSource source, Object id) {
314+
Object[] values = persister.getPropertyValuesToInsert(entity, getMergeMap(context), source);
315+
Type[] types = persister.getPropertyTypes();
316+
317+
boolean substitute = substituteValuesIfNecessary(entity, id, values, persister, source);
318+
if ( persister.hasCollections() ) {
319+
substitute = visitCollectionsBeforeSave(entity, id, values, types, source) || substitute;
320+
}
321+
322+
if ( substitute ) {
323+
persister.setValues(entity, values );
324+
}
325+
326+
TypeHelper.deepCopy(
327+
values,
328+
types,
329+
persister.getPropertyUpdateability(),
330+
values,
331+
source
332+
);
333+
return values;
321334
}
322335

323336
private AbstractEntityInsertAction addInsertAction(
@@ -434,7 +447,6 @@ protected void cascadeBeforeSave(
434447
EntityPersister persister,
435448
Object entity,
436449
C context) {
437-
438450
// cascade-save to many-to-one BEFORE the parent is saved
439451
final PersistenceContext persistenceContext = source.getPersistenceContextInternal();
440452
persistenceContext.incrementCascadeLevel();
@@ -466,9 +478,8 @@ protected void cascadeAfterSave(
466478
EntityPersister persister,
467479
Object entity,
468480
C context) {
469-
470-
final PersistenceContext persistenceContext = source.getPersistenceContextInternal();
471481
// cascade-save to collections AFTER the collection owner was saved
482+
final PersistenceContext persistenceContext = source.getPersistenceContextInternal();
472483
persistenceContext.incrementCascadeLevel();
473484
try {
474485
Cascade.cascade(

‎hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java‎

Lines changed: 21 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.hibernate.engine.spi.EntityEntry;
2828
import org.hibernate.engine.spi.EntityKey;
2929
import org.hibernate.engine.spi.PersistenceContext;
30-
import org.hibernate.engine.spi.SharedSessionContractImplementor;
3130
import org.hibernate.engine.spi.Status;
3231
import org.hibernate.event.service.spi.JpaBootstrapSensitive;
3332
import org.hibernate.event.spi.DeleteContext;
@@ -119,7 +118,7 @@ && canBeDeletedWithoutLoading( source, persister ) ) {
119118
if ( persister.hasOwnedCollections() ) {
120119
// we're deleting an unloaded proxy with collections
121120
for ( Type type : persister.getPropertyTypes() ) { //TODO: when we enable this for subclasses use getSubclassPropertyTypeClosure()
122-
deleteOwnedCollections( type, id, source, source.getActionQueue() );
121+
deleteOwnedCollections( type, id, source );
123122
}
124123
}
125124

@@ -132,8 +131,9 @@ && canBeDeletedWithoutLoading( source, persister ) ) {
132131
return false;
133132
}
134133

135-
private void deleteOwnedCollections(Type type, Object key, SharedSessionContractImplementor session, ActionQueue actionQueue) {
134+
private static void deleteOwnedCollections(Type type, Object key, EventSource session) {
136135
MappingMetamodelImplementor mappingMetamodel = session.getFactory().getMappingMetamodel();
136+
ActionQueue actionQueue = session.getActionQueue();
137137
if ( type.isCollectionType() ) {
138138
String role = ( (CollectionType) type ).getRole();
139139
CollectionPersister persister = mappingMetamodel.getCollectionDescriptor(role);
@@ -144,7 +144,7 @@ private void deleteOwnedCollections(Type type, Object key, SharedSessionContract
144144
else if ( type.isComponentType() ) {
145145
Type[] subtypes = ( (CompositeType) type ).getSubtypes();
146146
for ( Type subtype : subtypes ) {
147-
deleteOwnedCollections( subtype, key, session, actionQueue );
147+
deleteOwnedCollections( subtype, key, session );
148148
}
149149
}
150150
}
@@ -161,16 +161,12 @@ private void delete(DeleteEvent event, DeleteContext transientEntities) {
161161
}
162162
}
163163

164-
private void deleteTransientInstance(
165-
DeleteEvent event,
166-
DeleteContext transientEntities,
167-
Object entity) {
168-
164+
private void deleteTransientInstance(DeleteEvent event, DeleteContext transientEntities, Object entity) {
169165
LOG.trace( "Entity was not persistent in delete processing" );
170166

171167
final EventSource source = event.getSession();
172168

173-
EntityPersister persister = source.getEntityPersister(event.getEntityName(), entity);
169+
EntityPersister persister = source.getEntityPersister( event.getEntityName(), entity );
174170
if ( ForeignKeys.isTransient( persister.getEntityName(), entity, null, source ) ) {
175171
deleteTransientEntity( source, entity, event.isCascadeDeleteEnabled(), persister, transientEntities );
176172
}
@@ -194,7 +190,7 @@ private void deleteTransientInstance(
194190
EntityEntry entityEntry = persistenceContext.addEntity(
195191
entity,
196192
persister.isMutable() ? Status.MANAGED : Status.READ_ONLY,
197-
persister.getValues(entity),
193+
persister.getValues( entity ),
198194
key,
199195
version,
200196
LockMode.NONE,
@@ -213,11 +209,8 @@ private void deletePersistentInstance(
213209
DeleteContext transientEntities,
214210
Object entity,
215211
EntityEntry entityEntry) {
216-
217212
LOG.trace( "Deleting a persistent instance" );
218-
219213
final EventSource source = event.getSession();
220-
221214
if ( entityEntry.getStatus() == Status.DELETED || entityEntry.getStatus() == Status.GONE
222215
|| source.getPersistenceContextInternal()
223216
.containsDeletedUnloadedEntityKey( entityEntry.getEntityKey() ) ) {
@@ -245,24 +238,20 @@ private void delete(
245238
Object id,
246239
Object version,
247240
EntityEntry entityEntry) {
248-
249241
callbackRegistry.preRemove(entity);
250-
if ( invokeDeleteLifecycle(source, entity, persister) ) {
251-
return;
252-
}
253-
254-
deleteEntity(
255-
source,
256-
entity,
257-
entityEntry,
258-
event.isCascadeDeleteEnabled(),
259-
event.isOrphanRemovalBeforeUpdates(),
260-
persister,
261-
transientEntities
262-
);
263-
264-
if ( source.getFactory().getSessionFactoryOptions().isIdentifierRollbackEnabled() ) {
265-
persister.resetIdentifier(entity, id, version, source);
242+
if ( !invokeDeleteLifecycle( source, entity, persister ) ) {
243+
deleteEntity(
244+
source,
245+
entity,
246+
entityEntry,
247+
event.isCascadeDeleteEnabled(),
248+
event.isOrphanRemovalBeforeUpdates(),
249+
persister,
250+
transientEntities
251+
);
252+
if ( source.getFactory().getSessionFactoryOptions().isIdentifierRollbackEnabled() ) {
253+
persister.resetIdentifier( entity, id, version, source );
254+
}
266255
}
267256
}
268257

@@ -467,9 +456,7 @@ private Object[] createDeletedState(
467456
for ( int i = 0; i < types.length; i++) {
468457
if ( types[i].isCollectionType() && !enhancementMetadata.isAttributeLoaded( parent, propertyNames[i] ) ) {
469458
final CollectionType collectionType = (CollectionType) types[i];
470-
final CollectionPersister collectionDescriptor = persister.getFactory()
471-
.getRuntimeMetamodels()
472-
.getMappingMetamodel()
459+
final CollectionPersister collectionDescriptor = persister.getFactory().getMappingMetamodel()
473460
.getCollectionDescriptor( collectionType.getRole() );
474461
if ( collectionDescriptor.needsRemove() || collectionDescriptor.hasCache() ) {
475462
final Object keyOfOwner = collectionType.getKeyOfOwner( parent, eventSource.getSession() );

0 commit comments

Comments
 (0)