From 57056d91156ea5b8b79c8338f1784ce4354f08af Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 6 Dec 2025 11:25:49 +0100 Subject: [PATCH] misc cleanups in SessionImpl --- .../SharedStatelessSessionBuilderImpl.java | 2 +- .../AbstractSharedSessionContract.java | 3 +- .../org/hibernate/internal/SessionImpl.java | 111 +++++++++--------- .../hibernate/internal/SessionLogging.java | 4 + .../internal/StatelessSessionImpl.java | 4 +- ...leSharedStatelessSessionBuildingTests.java | 4 +- 6 files changed, 68 insertions(+), 60 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/creation/internal/SharedStatelessSessionBuilderImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/creation/internal/SharedStatelessSessionBuilderImpl.java index 585579dbf1a8..9af5a6c25f93 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/creation/internal/SharedStatelessSessionBuilderImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/creation/internal/SharedStatelessSessionBuilderImpl.java @@ -92,7 +92,7 @@ public SharedStatelessSessionBuilder interceptor() { @Override public SharedStatelessSessionBuilder statementInspector() { - this.statementInspector = original.getJdbcSessionContext().getStatementInspector(); + statementInspector = original.getJdbcSessionContext().getStatementInspector(); return this; } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java index 2474372bc614..907676485197 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java @@ -260,7 +260,8 @@ public SharedStatelessSessionBuilder statelessWithOptions() { @Override protected StatelessSessionImplementor createStatelessSession() { return new StatelessSessionImpl( factory, - new SessionCreationOptionsAdaptor( factory, this, AbstractSharedSessionContract.this ) ); + new SessionCreationOptionsAdaptor( factory, this, + AbstractSharedSessionContract.this ) ); } }; } 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 232f193263c5..148780126c9a 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -158,8 +158,8 @@ public class SessionImpl implements Serializable, SharedSessionContractImplementor, JdbcSessionOwner, SessionImplementor, EventSource, TransactionCoordinatorBuilder.Options, WrapperOptions, LoadAccessContext { - // Defaults to null which means the properties are the default - // as defined in FastSessionServices#defaultSessionProperties + // Defaults to null, meaning the properties + // are the default properties of the factory. private Map properties; private transient ActionQueue actionQueue; @@ -191,8 +191,6 @@ public SessionImpl(SessionFactoryImpl factory, SessionCreationOptions options) { actionQueue = createActionQueue(); eventListenerGroups = factory.getEventListenerGroups(); - flushMode = options.getInitialSessionFlushMode(); - autoClear = options.shouldAutoClear(); autoClose = options.shouldAutoClose(); @@ -202,13 +200,10 @@ public SessionImpl(SessionFactoryImpl factory, SessionCreationOptions options) { loadQueryInfluencers = new LoadQueryInfluencers( factory, options ); - // NOTE : pulse() already handles auto-join-ability correctly + // NOTE: pulse() already handles auto-join-ability correctly getTransactionCoordinator().pulse(); - // do not override explicitly set flush mode ( SessionBuilder#flushMode() ) - if ( getHibernateFlushMode() == null ) { - setHibernateFlushMode( getInitialFlushMode() ); - } + flushMode = getInitialFlushMode( options ); setUpMultitenancy( factory, loadQueryInfluencers ); @@ -247,10 +242,16 @@ private static void setUpTransactionCompletionProcesses( } } - private FlushMode getInitialFlushMode() { - return properties == null - ? getSessionFactoryOptions().getInitialSessionFlushMode() - : ConfigurationHelper.getFlushMode( getSessionProperty( HINT_FLUSH_MODE ), FlushMode.AUTO ); + private FlushMode getInitialFlushMode(SessionCreationOptions options) { + final var initialSessionFlushMode = options.getInitialSessionFlushMode(); + if ( initialSessionFlushMode != null ) { + return initialSessionFlushMode; + } + else { + return properties == null + ? getSessionFactoryOptions().getInitialSessionFlushMode() + : ConfigurationHelper.getFlushMode( properties.get( HINT_FLUSH_MODE ), FlushMode.AUTO ); + } } protected PersistenceContext createPersistenceContext(SessionCreationOptions options) { @@ -351,7 +352,6 @@ public void clear() { private void internalClear() { persistenceContext.clear(); actionQueue.clear(); - eventListenerGroups.eventListenerGroup_CLEAR .fireLazyEventOnEachListener( this::createClearEvent, ClearEventListener::onClear ); } @@ -392,7 +392,7 @@ public void closeWithoutOpenChecks() { else { // In the JPA bootstrap, if the session is closed // before the transaction commits, we just mark the - // session as closed, and set waitingForAutoClose. + // session as closed and set waitingForAutoClose. // This method will be called a second time from // afterTransactionCompletion when the transaction // commits, and the session will be closed for real. @@ -405,10 +405,12 @@ public void closeWithoutOpenChecks() { } } finally { - // E.g. when we are in the JTA context the session can get closed while the transaction is still active - // and JTA will call the AfterCompletion itself. Hence, we don't want to clear out the action queue callbacks at this point: - if ( !getTransactionCoordinator().isTransactionActive() && actionQueue.hasAfterTransactionActions() ) { - SESSION_LOGGER.warn( "Closing session with unprocessed clean up bulk operations, forcing their execution" ); + // E.g. When we are in the JTA context, the session can get closed while the + // transaction is still active and JTA will call the AfterCompletion itself. + // Hence, we don't want to clear out the action queue callbacks at this point: + if ( !getTransactionCoordinator().isTransactionActive() + && actionQueue.hasAfterTransactionActions() ) { + SESSION_LOGGER.closingSessionWithUnprocessedBulkOperations(); actionQueue.executePendingBulkOperationCleanUpActions(); } final var statistics = getSessionFactory().getStatistics(); @@ -489,8 +491,8 @@ else if ( isClosed() ) { return false; } else { - // JPA technically requires that this be a PersistentUnityTransactionType#JTA to work, - // but we do not assert that here: + // JPA requires PersistentUnitTransactionType.JTA, + // for this, but we do not assert that here: return isAutoCloseSessionEnabled(); // && getTransactionCoordinator().getTransactionCoordinatorBuilder().isJta(); } @@ -557,7 +559,9 @@ public Object getEntityUsingInterceptor(EntityKey key) { // logically, is PersistentContext the "thing" to which an interceptor gets attached? final Object result = persistenceContext.getEntity( key ); if ( result == null ) { - final Object newObject = getInterceptor().getEntity( key.getEntityName(), key.getIdentifier() ); + final Object newObject = + getInterceptor() + .getEntity( key.getEntityName(), key.getIdentifier() ); if ( newObject != null ) { lock( newObject, LockMode.NONE ); } @@ -870,8 +874,8 @@ public void removeOrphanBeforeUpdates(String entityName, Object child) { private void logRemoveOrphanBeforeUpdates(String timing, String entityName, Object entity) { if ( SESSION_LOGGER.isTraceEnabled() ) { final var entityEntry = persistenceContext.getEntry( entity ); - final String entityInfo = entityEntry == null ? entityName : infoString( entityName, entityEntry.getId() ); - SESSION_LOGGER.removeOrphanBeforeUpdates( timing, entityInfo ); + SESSION_LOGGER.removeOrphanBeforeUpdates( timing, + entityEntry == null ? entityName : infoString( entityName, entityEntry.getId() ) ); } } @@ -933,7 +937,7 @@ private void setMultiIdentifierLoadAccessOptions(FindOption[] options, Multi CacheRetrieveMode retrieveMode = getCacheRetrieveMode(); LockOptions lockOptions = copySessionLockOptions(); int batchSize = -1; - for ( FindOption option : options ) { + for ( var option : options ) { if ( option instanceof CacheStoreMode cacheStoreMode ) { storeMode = cacheStoreMode; } @@ -1553,7 +1557,8 @@ public void forceFlush(EntityEntry entityEntry) { @Override public void forceFlush(EntityKey key) { if ( SESSION_LOGGER.isTraceEnabled() ) { - SESSION_LOGGER.flushingToForceDeletion( infoString( key.getPersister(), key.getIdentifier(), getFactory() ) ); + SESSION_LOGGER.flushingToForceDeletion( + infoString( key.getPersister(), key.getIdentifier(), getFactory() ) ); } if ( persistenceContext.getCascadeLevel() > 0 ) { @@ -2124,7 +2129,7 @@ private boolean isTransactionFlushable() { return true; } else { - final TransactionStatus status = currentTransaction.getStatus(); + final var status = currentTransaction.getStatus(); return status == TransactionStatus.ACTIVE || status == TransactionStatus.COMMITTING; } @@ -2190,13 +2195,13 @@ private T find(Class entityClass, Object primaryKey, LockOptions lockOpti .load( primaryKey ); } catch ( FetchNotFoundException e ) { - // This may happen if the entity has an associations mapped with + // This may happen if the entity has an association mapped with // @NotFound(action = NotFoundAction.EXCEPTION) and this associated // entity is not found throw e; } catch ( EntityFilterException e ) { - // This may happen if the entity has an associations which is + // This may happen if the entity has an association which is // filtered by a FilterDef and this associated entity is not found throw e; } @@ -2256,7 +2261,7 @@ private void setLoadAccessOptions(FindOption[] options, IdentifierLoadAccess CacheStoreMode storeMode = getCacheStoreMode(); CacheRetrieveMode retrieveMode = getCacheRetrieveMode(); LockOptions lockOptions = copySessionLockOptions(); - for ( FindOption option : options ) { + for ( var option : options ) { if ( option instanceof CacheStoreMode cacheStoreMode ) { storeMode = cacheStoreMode; } @@ -2295,15 +2300,15 @@ else if ( option instanceof ReadOnlyMode ) { loadAccess.withReadOnly( option == ReadOnlyMode.READ_ONLY ); } else if ( option instanceof FindMultipleOption findMultipleOption ) { - throw new IllegalArgumentException( "Option '" + findMultipleOption + "' can only be used in 'findMultiple()'" ); + throw new IllegalArgumentException( "Option '" + findMultipleOption + + "' can only be used in 'findMultiple()'" ); } } - if ( lockOptions.getLockMode().isPessimistic() ) { - if ( lockOptions.getTimeOut() == WAIT_FOREVER_MILLI ) { - final Object factoryHint = getFactory().getProperties().get( HINT_SPEC_LOCK_TIMEOUT ); - if ( factoryHint != null ) { - lockOptions.setTimeOut( Timeouts.fromHint( factoryHint ) ); - } + if ( lockOptions.getLockMode().isPessimistic() + && lockOptions.getTimeOut() == WAIT_FOREVER_MILLI ) { + final Object factoryHint = getFactory().getProperties().get( HINT_SPEC_LOCK_TIMEOUT ); + if ( factoryHint != null ) { + lockOptions.setTimeOut( Timeouts.fromHint( factoryHint ) ); } } loadAccess.with( lockOptions ).with( interpretCacheMode( storeMode, retrieveMode ) ); @@ -2578,25 +2583,23 @@ public LockModeType getLockMode(Object entity) { @Override public void setProperty(String propertyName, Object value) { checkOpen(); - - if ( !( value instanceof Serializable ) ) { - SESSION_LOGGER.nonSerializableProperty( propertyName ); - return; - } - - if ( propertyName == null ) { - SESSION_LOGGER.nullPropertyKey(); - return; + if ( value instanceof Serializable ) { + if ( propertyName != null ) { // store property for future reference: + if ( properties == null ) { + properties = computeCurrentProperties(); + } + properties.put( propertyName, value ); + // now actually update the setting + // if it's one that affects this Session + interpretProperty( propertyName, value ); + } + else { + SESSION_LOGGER.nullPropertyKey(); + } } - - // store property for future reference: - if ( properties == null ) { - properties = computeCurrentProperties(); + else { + SESSION_LOGGER.nonSerializableProperty( propertyName ); } - properties.put( propertyName, value ); - - // now actually update the setting, if it's one which affects this Session - interpretProperty( propertyName, value ); } private void interpretProperty(String propertyName, Object value) { diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionLogging.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionLogging.java index cddf971d6999..a82a6fa12e56 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionLogging.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionLogging.java @@ -145,6 +145,10 @@ public interface SessionLogging extends BasicLogger { @Message(id = 90010107, value = "Exception in interceptor afterTransactionCompletion()") void exceptionInAfterTransactionCompletionInterceptor(@Cause Throwable e); + @LogMessage(level = WARN) + @Message(id = 90010108, value = "Closing session with unprocessed clean up bulk operations, forcing their execution") + void closingSessionWithUnprocessedBulkOperations(); + // StatelessSession-specific @LogMessage(level = TRACE) diff --git a/hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java index f38757327a72..778a14a1c543 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java @@ -152,8 +152,8 @@ public StatelessSessionImpl(SessionFactoryImpl factory, SessionCreationOptions o influencers = new LoadQueryInfluencers( getFactory() ); eventListenerGroups = factory.getEventListenerGroups(); setUpMultitenancy( factory, influencers ); - // a nonzero batch size forces use of write-behind - // therefore ignore the value of hibernate.jdbc.batch_size + // A nonzero batch size forces the use of write-behind + // Therefore, ignore the value of hibernate.jdbc.batch_size setJdbcBatchSize( 0 ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/shared/SimpleSharedStatelessSessionBuildingTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/shared/SimpleSharedStatelessSessionBuildingTests.java index 39764908cb59..6bbb89e18bc4 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/shared/SimpleSharedStatelessSessionBuildingTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/shared/SimpleSharedStatelessSessionBuildingTests.java @@ -135,9 +135,9 @@ void testUsage(SessionFactoryScope factoryScope) { session.insert( new Something( 2, "first" ) ); assertSame( session.getTransaction(), statelessSession.getTransaction() ); assertSame( ((StatelessSessionImplementor) session).getJdbcCoordinator(), - ((StatelessSessionImplementor) statelessSession).getJdbcCoordinator() ); + statelessSession.getJdbcCoordinator() ); assertSame( ((StatelessSessionImplementor) session).getTransactionCompletionCallbacksImplementor(), - ((StatelessSessionImplementor) statelessSession).getTransactionCompletionCallbacksImplementor() ); + statelessSession.getTransactionCompletionCallbacksImplementor() ); } } ); assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );