From 453f0ff0749136eac008f9c16fb7742de7ccbadb Mon Sep 17 00:00:00 2001 From: Gavin King Date: Wed, 20 Nov 2024 10:50:01 +0100 Subject: [PATCH] improve exception messages and logging related to tx management along with some minor aesthetic code cleanups --- .../main/java/org/hibernate/Transaction.java | 12 +- .../transaction/internal/TransactionImpl.java | 139 ++++++++---------- .../internal/ExceptionConverterImpl.java | 2 +- .../internal/FastSessionServices.java | 14 +- .../internal/MutableJpaComplianceImpl.java | 8 +- ...sourceLocalTransactionCoordinatorImpl.java | 59 ++++---- .../JtaTransactionCoordinatorImpl.java | 112 +++++++------- .../spi/TransactionCoordinatorBuilder.java | 4 +- .../TransactionCommitFailureTest.java | 3 +- .../tool/hbm2ddl/SchemaUpdateTask.java | 2 +- .../tool/hbm2ddl/SchemaValidatorTask.java | 2 +- 11 files changed, 161 insertions(+), 196 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/Transaction.java b/hibernate-core/src/main/java/org/hibernate/Transaction.java index 814a92bd35..0ce4b4f037 100644 --- a/hibernate-core/src/main/java/org/hibernate/Transaction.java +++ b/hibernate-core/src/main/java/org/hibernate/Transaction.java @@ -69,9 +69,13 @@ public interface Transaction extends EntityTransaction { /** * Attempt to mark the underlying transaction for rollback only. + *

+ * Unlike {@link #setRollbackOnly()}, which is specified by JPA + * to throw when the transaction is inactive, this operation may + * be called on an inactive transaction, in which case it has no + * effect. + * + * @see #setRollbackOnly() */ - default void markRollbackOnly() { - setRollbackOnly(); - } - + void markRollbackOnly(); } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionImpl.java index a94e81cf4a..ebd603d052 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionImpl.java @@ -12,7 +12,6 @@ import org.hibernate.TransactionException; import org.hibernate.engine.transaction.spi.TransactionImplementor; import org.hibernate.internal.AbstractSharedSessionContract; import org.hibernate.internal.CoreLogging; -import org.hibernate.jpa.spi.JpaCompliance; import org.hibernate.resource.transaction.spi.TransactionCoordinator; import org.hibernate.resource.transaction.spi.TransactionStatus; @@ -28,7 +27,7 @@ public class TransactionImpl implements TransactionImplementor { private static final Logger LOG = CoreLogging.logger( TransactionImpl.class ); private final TransactionCoordinator transactionCoordinator; - private final JpaCompliance jpaCompliance; + private final boolean jpaCompliance; private final AbstractSharedSessionContract session; private TransactionDriver transactionDriverControl; @@ -37,7 +36,9 @@ public class TransactionImpl implements TransactionImplementor { TransactionCoordinator transactionCoordinator, AbstractSharedSessionContract session) { this.transactionCoordinator = transactionCoordinator; - this.jpaCompliance = session.getFactory().getSessionFactoryOptions().getJpaCompliance(); + this.jpaCompliance = + session.getFactory().getSessionFactoryOptions().getJpaCompliance() + .isJpaTransactionComplianceEnabled(); this.session = session; if ( session.isOpen() && transactionCoordinator.isActive() ) { @@ -47,11 +48,8 @@ public class TransactionImpl implements TransactionImplementor { LOG.debug( "TransactionImpl created on closed Session/EntityManager" ); } - if ( LOG.isDebugEnabled() ) { - LOG.debugf( - "On TransactionImpl creation, JpaCompliance#isJpaTransactionComplianceEnabled == %s", - jpaCompliance.isJpaTransactionComplianceEnabled() - ); + if ( LOG.isDebugEnabled() && jpaCompliance ) { + LOG.debugf( "TransactionImpl created in JPA compliant mode" ); } } @@ -65,78 +63,66 @@ public class TransactionImpl implements TransactionImplementor { transactionDriverControl = transactionCoordinator.getTransactionDriverControl(); } - // per-JPA if ( isActive() ) { - if ( jpaCompliance.isJpaTransactionComplianceEnabled() - || !transactionCoordinator.getTransactionCoordinatorBuilder().isJta() ) { - throw new IllegalStateException( "Transaction already active" ); + if ( jpaCompliance ) { + throw new IllegalStateException( "Transaction already active (in JPA compliant mode)" ); } - else { - return; + else if ( !transactionCoordinator.getTransactionCoordinatorBuilder().isJta() ) { + throw new IllegalStateException( "Resource-local transaction already active" ); } } - - LOG.debug( "begin" ); - - this.transactionDriverControl.begin(); + else { + LOG.debug( "begin transaction" ); + transactionDriverControl.begin(); + } } @Override public void commit() { + // allow MARKED_ROLLBACK to propagate through to transactionDriverControl + // the boolean passed to isActive indicates whether MARKED_ROLLBACK should + // be considered active if ( !isActive( true ) ) { - // allow MARKED_ROLLBACK to propagate through to transactionDriverControl - // the boolean passed to isActive indicates whether MARKED_ROLLBACK should be - // considered active - // - // essentially here we have a transaction that is not active and - // has not been marked for rollback only + // we have a transaction that is inactive and has not been marked for rollback only throw new IllegalStateException( "Transaction not successfully started" ); } - - LOG.debug( "committing" ); - - try { - internalGetTransactionDriverControl().commit(); - } - catch (RuntimeException e) { - throw session.getExceptionConverter().convertCommitException( e ); + else { + LOG.debug( "committing transaction" ); + try { + internalGetTransactionDriverControl().commit(); + } + catch (RuntimeException e) { + throw session.getExceptionConverter().convertCommitException( e ); + } } } public TransactionDriver internalGetTransactionDriverControl() { // NOTE here to help be a more descriptive NullPointerException - if ( this.transactionDriverControl == null ) { + if ( transactionDriverControl == null ) { throw new IllegalStateException( "Transaction was not properly begun/started" ); } - return this.transactionDriverControl; + else { + return transactionDriverControl; + } } @Override public void rollback() { - if ( !isActive() ) { - if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) { - - throw new IllegalStateException( - "JPA compliance dictates throwing IllegalStateException when #rollback " + - "is called on non-active transaction" - ); - } + if ( !isActive() && jpaCompliance ) { + throw new IllegalStateException( "rollback() called on inactive transaction (in JPA compliant mode)" ); } - TransactionStatus status = getStatus(); + final TransactionStatus status = getStatus(); if ( status == TransactionStatus.ROLLED_BACK || status == TransactionStatus.NOT_ACTIVE ) { - // Allow rollback() calls on completed transactions, just no-op. + // allow rollback() on completed transaction as noop LOG.debug( "rollback() called on an inactive transaction" ); - return; } - - if ( !status.canRollback() ) { - throw new TransactionException( "Cannot rollback transaction in current status [" + status.name() + "]" ); + else if ( !status.canRollback() ) { + throw new TransactionException( "Cannot roll back transaction in current status [" + status.name() + "]" ); } - - LOG.debug( "rolling back" ); - - if ( status != TransactionStatus.FAILED_COMMIT || allowFailedCommitToPhysicallyRollback() ) { + else if ( status != TransactionStatus.FAILED_COMMIT || allowFailedCommitToPhysicallyRollback() ) { + LOG.debug( "rolling back transaction" ); internalGetTransactionDriverControl().rollback(); } } @@ -176,55 +162,50 @@ public class TransactionImpl implements TransactionImplementor { @Override public void registerSynchronization(Synchronization synchronization) throws HibernateException { - this.transactionCoordinator.getLocalSynchronizations().registerSynchronization( synchronization ); + transactionCoordinator.getLocalSynchronizations().registerSynchronization( synchronization ); } @Override public void setTimeout(int seconds) { - this.transactionCoordinator.setTimeOut( seconds ); + transactionCoordinator.setTimeOut( seconds ); } @Override public void setTimeout(@Nullable Integer seconds) { - this.transactionCoordinator.setTimeOut( seconds == null ? -1 : seconds ); + transactionCoordinator.setTimeOut( seconds == null ? -1 : seconds ); } @Override public @Nullable Integer getTimeout() { - final int timeOut = this.transactionCoordinator.getTimeOut(); - if ( timeOut == -1 ) { - return null; - } - return timeOut; + final int timeout = transactionCoordinator.getTimeOut(); + return timeout == -1 ? null : timeout; } @Override public void markRollbackOnly() { - // this is the Hibernate-specific API, whereas #setRollbackOnly is the - // JPA-defined API. In our opinion it is much more user-friendly to + // this is the Hibernate-specific API, whereas setRollbackOnly is the + // JPA-defined API. In our opinion it is much more user-friendly to // always allow user/integration to indicate that the transaction // should not be allowed to commit. // - // However.. should only "do something" on an active transaction if ( isActive() ) { internalGetTransactionDriverControl().markRollbackOnly(); } + // else noop for an inactive transaction } @Override public void setRollbackOnly() { if ( !isActive() ) { - // Since this is the JPA-defined one, we make sure the txn is active first - // so long as compliance (JpaCompliance) has not been defined to disable - // that check - making this active more like Hibernate's #markRollbackOnly - if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) { - throw new IllegalStateException( - "JPA compliance dictates throwing IllegalStateException when #setRollbackOnly " + - "is called on non-active transaction" - ); + if ( jpaCompliance ) { + // This is the JPA-defined version of this operation, + // so we must check that the transaction is active + throw new IllegalStateException( "setRollbackOnly() called on inactive transaction (in JPA compliant mode)" ); } else { - LOG.debug( "#setRollbackOnly called on a not-active transaction" ); + // JpaCompliance disables the check, so this method + // is equivalent our native markRollbackOnly() + LOG.debug( "setRollbackOnly() called on a inactive transaction" ); } } else { @@ -234,16 +215,12 @@ public class TransactionImpl implements TransactionImplementor { @Override public boolean getRollbackOnly() { - if ( !isActive() ) { - if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) { - throw new IllegalStateException( - "JPA compliance dictates throwing IllegalStateException when #getRollbackOnly " + - "is called on non-active transaction" - ); - } + if ( jpaCompliance && !isActive() ) { + throw new IllegalStateException( "getRollbackOnly() called on inactive transaction (in JPA compliant mode)" ); + } + else { + return getStatus() == TransactionStatus.MARKED_ROLLBACK; } - - return getStatus() == TransactionStatus.MARKED_ROLLBACK; } protected boolean allowFailedCommitToPhysicallyRollback() { diff --git a/hibernate-core/src/main/java/org/hibernate/internal/ExceptionConverterImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/ExceptionConverterImpl.java index dbbf1b721d..c68e9576c4 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/ExceptionConverterImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/ExceptionConverterImpl.java @@ -63,7 +63,7 @@ public class ExceptionConverterImpl implements ExceptionConverter { catch (Exception re) { //swallow } - return new RollbackException( "Error while committing the transaction", + return new RollbackException( "Error while committing the transaction [" + exception.getMessage() + "]", exception instanceof HibernateException hibernateException ? convert( hibernateException ) : exception ); diff --git a/hibernate-core/src/main/java/org/hibernate/internal/FastSessionServices.java b/hibernate-core/src/main/java/org/hibernate/internal/FastSessionServices.java index 8880bc3586..33aea1b209 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/FastSessionServices.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/FastSessionServices.java @@ -358,18 +358,16 @@ public final class FastSessionServices { private static CacheRetrieveMode determineCacheRetrieveMode(Map settings) { final CacheRetrieveMode cacheRetrieveMode = (CacheRetrieveMode) settings.get( JPA_SHARED_CACHE_RETRIEVE_MODE ); - if ( cacheRetrieveMode == null ) { - return (CacheRetrieveMode) settings.get( JAKARTA_SHARED_CACHE_RETRIEVE_MODE ); - } - return cacheRetrieveMode; + return cacheRetrieveMode == null + ? (CacheRetrieveMode) settings.get( JAKARTA_SHARED_CACHE_RETRIEVE_MODE ) + : cacheRetrieveMode; } private static CacheStoreMode determineCacheStoreMode(Map settings) { final CacheStoreMode cacheStoreMode = (CacheStoreMode) settings.get( JPA_SHARED_CACHE_STORE_MODE ); - if ( cacheStoreMode == null ) { - return ( CacheStoreMode ) settings.get( JAKARTA_SHARED_CACHE_STORE_MODE ); - } - return cacheStoreMode; + return cacheStoreMode == null + ? (CacheStoreMode) settings.get( JAKARTA_SHARED_CACHE_STORE_MODE ) + : cacheStoreMode; } public JdbcValuesMappingProducerProvider getJdbcValuesMappingProducerProvider() { diff --git a/hibernate-core/src/main/java/org/hibernate/jpa/internal/MutableJpaComplianceImpl.java b/hibernate-core/src/main/java/org/hibernate/jpa/internal/MutableJpaComplianceImpl.java index c42bdcc594..9e0313a288 100644 --- a/hibernate-core/src/main/java/org/hibernate/jpa/internal/MutableJpaComplianceImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/jpa/internal/MutableJpaComplianceImpl.java @@ -169,15 +169,15 @@ public class MutableJpaComplianceImpl implements MutableJpaCompliance { @Override public JpaCompliance immutableCopy() { - JpaComplianceImpl.JpaComplianceBuilder builder = new JpaComplianceImpl.JpaComplianceBuilder(); - builder = builder.setProxyCompliance( proxyCompliance ) + return new JpaComplianceImpl.JpaComplianceBuilder() + .setProxyCompliance( proxyCompliance ) .setOrderByMappingCompliance( orderByMappingCompliance ) .setGlobalGeneratorNameCompliance( generatorNameScopeCompliance ) .setQueryCompliance( queryCompliance ) .setTransactionCompliance( transactionCompliance ) .setClosedCompliance( closedCompliance ) .setCachingCompliance( cachingCompliance ) - .setLoadByIdCompliance( loadByIdCompliance ); - return builder.createJpaCompliance(); + .setLoadByIdCompliance( loadByIdCompliance ) + .createJpaCompliance(); } } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/JdbcResourceLocalTransactionCoordinatorImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/JdbcResourceLocalTransactionCoordinatorImpl.java index 07129285b8..1a5449c639 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/JdbcResourceLocalTransactionCoordinatorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/JdbcResourceLocalTransactionCoordinatorImpl.java @@ -148,7 +148,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC transactionCoordinatorOwner.setTransactionTimeOut( timeOut ); } - // report entering into a "transactional context" transactionCoordinatorOwner.startTransactionBoundary(); @@ -184,7 +183,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC log.tracef( "ResourceLocalTransactionCoordinatorImpl#afterCompletionCallback(%s)", successful ); final int statusToSend = successful ? Status.STATUS_COMMITTED : Status.STATUS_UNKNOWN; synchronizationRegistry.notifySynchronizationsAfterTransactionCompletion( statusToSend ); - transactionCoordinatorOwner.afterTransactionCompletion( successful, false ); for ( TransactionObserver observer : observers() ) { observer.afterCompletion( successful, false ); @@ -216,7 +214,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC private boolean rollbackOnly = false; public TransactionDriverControlImpl(JdbcResourceTransaction jdbcResourceTransaction) { - super(); this.jdbcResourceTransaction = jdbcResourceTransaction; } @@ -227,7 +224,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC @Override public void begin() { errorIfInvalid(); - jdbcResourceTransaction.begin(); JdbcResourceLocalTransactionCoordinatorImpl.this.afterBeginCallback(); } @@ -242,30 +238,13 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC public void commit() { try { if ( rollbackOnly ) { - log.debug( "On commit, transaction was marked for roll-back only, rolling back" ); - - try { - rollback(); - - if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) { - log.debug( "Throwing RollbackException on roll-back of transaction marked rollback-only on commit" ); - throw new RollbackException( "Transaction was marked for rollback-only" ); - } - - return; - } - catch (RollbackException e) { - throw e; - } - catch (RuntimeException e) { - log.debug( "Encountered failure rolling back failed commit", e ); - throw e; - } + commitRollbackOnly(); + } + else { + JdbcResourceLocalTransactionCoordinatorImpl.this.beforeCompletionCallback(); + jdbcResourceTransaction.commit(); + JdbcResourceLocalTransactionCoordinatorImpl.this.afterCompletionCallback( true ); } - - JdbcResourceLocalTransactionCoordinatorImpl.this.beforeCompletionCallback(); - jdbcResourceTransaction.commit(); - JdbcResourceLocalTransactionCoordinatorImpl.this.afterCompletionCallback( true ); } catch (RollbackException e) { throw e; @@ -281,6 +260,23 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC } } + private void commitRollbackOnly() { + log.debug( "On commit, transaction was marked for roll-back only, rolling back" ); + try { + rollback(); + if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) { + throw new RollbackException( "Transaction was marked for rollback-only" ); + } + } + catch (RollbackException e) { + throw e; + } + catch (RuntimeException e) { + log.debug( "Encountered failure rolling back failed commit", e ); + throw e; + } + } + @Override public void rollback() { try { @@ -292,8 +288,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC finally { rollbackOnly = false; } - - // no-op otherwise. } @Override @@ -305,12 +299,9 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC public void markRollbackOnly() { if ( getStatus() != TransactionStatus.ROLLED_BACK ) { if ( log.isDebugEnabled() ) { - log.debug( - "JDBC transaction marked for rollback-only (exception provided for stack trace)", - new Exception( "exception just for purpose of providing stack trace" ) - ); + log.debug( "JDBC transaction marked for rollback-only (exception provided for stack trace)", + new Exception( "exception just for purpose of providing stack trace" ) ); } - rollbackOnly = true; } } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaTransactionCoordinatorImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaTransactionCoordinatorImpl.java index e2deb02fee..ab1a4f63e0 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaTransactionCoordinatorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaTransactionCoordinatorImpl.java @@ -136,21 +136,15 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy @Override public void pulse() { - if ( !autoJoinTransactions ) { - return; + if ( autoJoinTransactions && !synchronizationRegistered ) { + // Can we register a synchronization according to the JtaPlatform? + if ( !jtaPlatform.canRegisterSynchronization() ) { + log.trace( "JTA platform says we cannot currently register synchronization; skipping" ); + } + else { + joinJtaTransaction(); + } } - - if ( synchronizationRegistered ) { - return; - } - - // Can we register a synchronization according to the JtaPlatform? - if ( !jtaPlatform.canRegisterSynchronization() ) { - log.trace( "JTA platform says we cannot currently register synchronization; skipping" ); - return; - } - - joinJtaTransaction(); } /** @@ -158,33 +152,30 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy * RegisteredSynchronization with the JTA system */ private void joinJtaTransaction() { - if ( synchronizationRegistered ) { - return; + if ( !synchronizationRegistered ) { + jtaPlatform.registerSynchronization( + new RegisteredSynchronization( getSynchronizationCallbackCoordinator() ) ); + getSynchronizationCallbackCoordinator().synchronizationRegistered(); + synchronizationRegistered = true; + log.debug( "Hibernate RegisteredSynchronization successfully registered with JTA platform" ); + // report entering into a "transactional context" + getTransactionCoordinatorOwner().startTransactionBoundary(); } - - jtaPlatform.registerSynchronization( new RegisteredSynchronization( getSynchronizationCallbackCoordinator() ) ); - getSynchronizationCallbackCoordinator().synchronizationRegistered(); - synchronizationRegistered = true; - log.debug( "Hibernate RegisteredSynchronization successfully registered with JTA platform" ); - - // report entering into a "transactional context" - getTransactionCoordinatorOwner().startTransactionBoundary(); } @Override public void explicitJoin() { if ( synchronizationRegistered ) { log.debug( "JTA transaction was already joined (RegisteredSynchronization already registered)" ); - return; } - - if ( getTransactionDriverControl().getStatus() != ACTIVE ) { - throw new TransactionRequiredForJoinException( - "Explicitly joining a JTA transaction requires a JTA transaction be currently active" - ); + else { + if ( getTransactionDriverControl().getStatus() != ACTIVE ) { + throw new TransactionRequiredForJoinException( + "Explicitly joining a JTA transaction requires a JTA transaction be currently active" + ); + } + joinJtaTransaction(); } - - joinJtaTransaction(); } @Override @@ -204,7 +195,7 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy } public TransactionCoordinatorOwner getTransactionCoordinatorOwner(){ - return this.transactionCoordinatorOwner; + return transactionCoordinatorOwner; } @Override @@ -221,32 +212,36 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy } private TransactionDriverControlImpl makePhysicalTransactionDelegate() { - JtaTransactionAdapter adapter; - - if ( preferUserTransactions ) { - adapter = makeUserTransactionAdapter(); - - if ( adapter == null ) { - log.debug( "Unable to access UserTransaction, attempting to use TransactionManager instead" ); - adapter = makeTransactionManagerAdapter(); - } - } - else { - adapter = makeTransactionManagerAdapter(); - - if ( adapter == null ) { - log.debug( "Unable to access TransactionManager, attempting to use UserTransaction instead" ); - adapter = makeUserTransactionAdapter(); - } - } - + final JtaTransactionAdapter adapter = + preferUserTransactions + ? getTransactionAdapterPreferringUserTransaction() + : getTransactionAdapterPreferringTransactionManager(); if ( adapter == null ) { throw new JtaPlatformInaccessibleException( "Unable to access TransactionManager or UserTransaction to make physical transaction delegate" ); } + else { + return new TransactionDriverControlImpl( adapter ); + } + } - return new TransactionDriverControlImpl( adapter ); + private JtaTransactionAdapter getTransactionAdapterPreferringTransactionManager() { + final JtaTransactionAdapter adapter = makeTransactionManagerAdapter(); + if ( adapter == null ) { + log.debug( "Unable to access TransactionManager, attempting to use UserTransaction instead" ); + return makeUserTransactionAdapter(); + } + return adapter; + } + + private JtaTransactionAdapter getTransactionAdapterPreferringUserTransaction() { + final JtaTransactionAdapter adapter = makeUserTransactionAdapter(); + if ( adapter == null ) { + log.debug( "Unable to access UserTransaction, attempting to use TransactionManager instead" ); + return makeTransactionManagerAdapter(); + } + return adapter; } private JtaTransactionAdapter makeUserTransactionAdapter() { @@ -254,6 +249,7 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy final UserTransaction userTransaction = jtaPlatform.retrieveUserTransaction(); if ( userTransaction == null ) { log.debug( "JtaPlatform#retrieveUserTransaction returned null" ); + return null; } else { return new JtaTransactionAdapterUserTransactionImpl( userTransaction ); @@ -261,9 +257,8 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy } catch ( Exception exception ) { log.debugf( "JtaPlatform#retrieveUserTransaction threw an exception [%s]", exception.getMessage() ); + return null; } - - return null; } private JtaTransactionAdapter makeTransactionManagerAdapter() { @@ -271,6 +266,7 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy final TransactionManager transactionManager = jtaPlatform.retrieveTransactionManager(); if ( transactionManager == null ) { log.debug( "JtaPlatform#retrieveTransactionManager returned null" ); + return null; } else { return new JtaTransactionAdapterTransactionManagerImpl( transactionManager ); @@ -278,9 +274,8 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy } catch ( Exception exception ) { log.debugf( "JtaPlatform#retrieveTransactionManager threw an exception [%s]", exception.getMessage() ); + return null; } - - return null; } @Override @@ -399,7 +394,6 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy @Override public void begin() { errorIfInvalid(); - jtaTransactionAdapter.begin(); JtaTransactionCoordinatorImpl.this.joinJtaTransaction(); } @@ -414,7 +408,6 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy public void commit() { errorIfInvalid(); getTransactionCoordinatorOwner().flushBeforeTransactionCompletion(); - // we don't have to perform any before/after completion processing here. We leave that for // the Synchronization callbacks jtaTransactionAdapter.commit(); @@ -423,7 +416,6 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy @Override public void rollback() { errorIfInvalid(); - // we don't have to perform any after completion processing here. We leave that for // the Synchronization callbacks jtaTransactionAdapter.rollback(); diff --git a/hibernate-core/src/main/java/org/hibernate/resource/transaction/spi/TransactionCoordinatorBuilder.java b/hibernate-core/src/main/java/org/hibernate/resource/transaction/spi/TransactionCoordinatorBuilder.java index 23b4ade5dd..fa80af22c1 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/transaction/spi/TransactionCoordinatorBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/transaction/spi/TransactionCoordinatorBuilder.java @@ -40,6 +40,8 @@ public interface TransactionCoordinatorBuilder extends Service { PhysicalConnectionHandlingMode getDefaultConnectionHandlingMode(); default DdlTransactionIsolator buildDdlTransactionIsolator(JdbcContext jdbcContext) { - return isJta() ? new DdlTransactionIsolatorJtaImpl( jdbcContext ) : new DdlTransactionIsolatorNonJtaImpl( jdbcContext ); + return isJta() + ? new DdlTransactionIsolatorJtaImpl( jdbcContext ) + : new DdlTransactionIsolatorNonJtaImpl( jdbcContext ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/transaction/TransactionCommitFailureTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/transaction/TransactionCommitFailureTest.java index ad434168f2..dbe854c220 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/transaction/TransactionCommitFailureTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/transaction/TransactionCommitFailureTest.java @@ -31,6 +31,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; /** @@ -73,7 +74,7 @@ public class TransactionCommitFailureTest { em.getTransaction().commit(); } catch (RollbackException e) { - assertEquals( COMMIT_FAILURE, e.getLocalizedMessage() ); + assertTrue( e.getLocalizedMessage().startsWith( COMMIT_FAILURE ) ); } finally { if ( em.getTransaction() != null && em.getTransaction().isActive() ) { diff --git a/tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/SchemaUpdateTask.java b/tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/SchemaUpdateTask.java index e1ec88c059..7f144666ce 100644 --- a/tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/SchemaUpdateTask.java +++ b/tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/SchemaUpdateTask.java @@ -193,7 +193,7 @@ public class SchemaUpdateTask extends MatchingTask { throw new BuildException( "File not found: " + e.getMessage(), e ); } catch (IOException e) { - throw new BuildException( "IOException : " + e.getMessage(), e ); + throw new BuildException( "IOException: " + e.getMessage(), e ); } catch (BuildException e) { throw e; diff --git a/tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/SchemaValidatorTask.java b/tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/SchemaValidatorTask.java index 5d1558d08d..ac2c8601aa 100644 --- a/tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/SchemaValidatorTask.java +++ b/tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/SchemaValidatorTask.java @@ -135,7 +135,7 @@ public class SchemaValidatorTask extends MatchingTask { throw new BuildException("File not found: " + e.getMessage(), e); } catch (IOException e) { - throw new BuildException("IOException : " + e.getMessage(), e); + throw new BuildException("IOException: " + e.getMessage(), e); } catch (BuildException e) { throw e;