Allows for programmatic management of JDBC 3.0 - * {@link java.sql.Savepoint Savepoints}. Spring's - * {@link org.springframework.transaction.support.DefaultTransactionStatus} - * will automatically delegate to this, as it autodetects transaction - * objects that implement the SavepointManager interface. + *
Allows for programmatic management of JDBC {@link java.sql.Savepoint Savepoints}. + * Spring's {@link org.springframework.transaction.support.DefaultTransactionStatus} + * automatically delegates to this, as it autodetects transaction objects which + * implement the {@link SavepointManager} interface. * * @author Juergen Hoeller * @since 1.1 + * @see DataSourceTransactionManager */ public abstract class JdbcTransactionObjectSupport implements SavepointManager, SmartTransactionObject { @@ -107,6 +106,10 @@ public abstract class JdbcTransactionObjectSupport implements SavepointManager, throw new NestedTransactionNotSupportedException( "Cannot create a nested transaction because savepoints are not supported by your JDBC driver"); } + if (conHolder.isRollbackOnly()) { + throw new CannotCreateTransactionException( + "Cannot create savepoint for transaction which is already marked as rollback-only"); + } return conHolder.createSavepoint(); } catch (SQLException ex) { @@ -123,6 +126,7 @@ public abstract class JdbcTransactionObjectSupport implements SavepointManager, ConnectionHolder conHolder = getConnectionHolderForSavepoint(); try { conHolder.getConnection().rollback((Savepoint) savepoint); + conHolder.resetRollbackOnly(); } catch (Throwable ex) { throw new TransactionSystemException("Could not roll back to JDBC savepoint", ex); @@ -151,7 +155,7 @@ public abstract class JdbcTransactionObjectSupport implements SavepointManager, } if (!hasConnectionHolder()) { throw new TransactionUsageException( - "Cannot create nested transaction if not exposing a JDBC transaction"); + "Cannot create nested transaction when not exposing a JDBC transaction"); } return getConnectionHolder(); } diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java index f527921e9a6..ac836ce7a9a 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java @@ -1369,6 +1369,122 @@ public class DataSourceTransactionManagerTests { verify(con).close(); } + @Test + public void testExistingTransactionWithPropagationNestedAndRequiredRollback() throws Exception { + DatabaseMetaData md = mock(DatabaseMetaData.class); + Savepoint sp = mock(Savepoint.class); + + given(md.supportsSavepoints()).willReturn(true); + given(con.getMetaData()).willReturn(md); + given(con.setSavepoint("SAVEPOINT_1")).willReturn(sp); + + final TransactionTemplate tt = new TransactionTemplate(tm); + tt.setPropagationBehavior(TransactionDefinition.PROPAGATION_NESTED); + assertTrue("Hasn't thread connection", !TransactionSynchronizationManager.hasResource(ds)); + assertTrue("Synchronization not active", !TransactionSynchronizationManager.isSynchronizationActive()); + + tt.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) throws RuntimeException { + assertTrue("Is new transaction", status.isNewTransaction()); + assertTrue("Isn't nested transaction", !status.hasSavepoint()); + try { + tt.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) throws RuntimeException { + assertTrue("Has thread connection", TransactionSynchronizationManager.hasResource(ds)); + assertTrue("Synchronization active", TransactionSynchronizationManager.isSynchronizationActive()); + assertTrue("Isn't new transaction", !status.isNewTransaction()); + assertTrue("Is nested transaction", status.hasSavepoint()); + TransactionTemplate ntt = new TransactionTemplate(tm); + ntt.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) throws RuntimeException { + assertTrue("Has thread connection", TransactionSynchronizationManager.hasResource(ds)); + assertTrue("Synchronization active", TransactionSynchronizationManager.isSynchronizationActive()); + assertTrue("Isn't new transaction", !status.isNewTransaction()); + assertTrue("Is regular transaction", !status.hasSavepoint()); + throw new IllegalStateException(); + } + }); + } + }); + fail("Should have thrown IllegalStateException"); + } + catch (IllegalStateException ex) { + // expected + } + assertTrue("Is new transaction", status.isNewTransaction()); + assertTrue("Isn't nested transaction", !status.hasSavepoint()); + } + }); + + assertTrue("Hasn't thread connection", !TransactionSynchronizationManager.hasResource(ds)); + verify(con).rollback(sp); + verify(con).releaseSavepoint(sp); + verify(con).commit(); + verify(con).isReadOnly(); + verify(con).close(); + } + + @Test + public void testExistingTransactionWithPropagationNestedAndRequiredRollbackOnly() throws Exception { + DatabaseMetaData md = mock(DatabaseMetaData.class); + Savepoint sp = mock(Savepoint.class); + + given(md.supportsSavepoints()).willReturn(true); + given(con.getMetaData()).willReturn(md); + given(con.setSavepoint("SAVEPOINT_1")).willReturn(sp); + + final TransactionTemplate tt = new TransactionTemplate(tm); + tt.setPropagationBehavior(TransactionDefinition.PROPAGATION_NESTED); + assertTrue("Hasn't thread connection", !TransactionSynchronizationManager.hasResource(ds)); + assertTrue("Synchronization not active", !TransactionSynchronizationManager.isSynchronizationActive()); + + tt.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) throws RuntimeException { + assertTrue("Is new transaction", status.isNewTransaction()); + assertTrue("Isn't nested transaction", !status.hasSavepoint()); + try { + tt.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) throws RuntimeException { + assertTrue("Has thread connection", TransactionSynchronizationManager.hasResource(ds)); + assertTrue("Synchronization active", TransactionSynchronizationManager.isSynchronizationActive()); + assertTrue("Isn't new transaction", !status.isNewTransaction()); + assertTrue("Is nested transaction", status.hasSavepoint()); + TransactionTemplate ntt = new TransactionTemplate(tm); + ntt.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) throws RuntimeException { + assertTrue("Has thread connection", TransactionSynchronizationManager.hasResource(ds)); + assertTrue("Synchronization active", TransactionSynchronizationManager.isSynchronizationActive()); + assertTrue("Isn't new transaction", !status.isNewTransaction()); + assertTrue("Is regular transaction", !status.hasSavepoint()); + status.setRollbackOnly(); + } + }); + } + }); + fail("Should have thrown UnexpectedRollbackException"); + } + catch (UnexpectedRollbackException ex) { + // expected + } + assertTrue("Is new transaction", status.isNewTransaction()); + assertTrue("Isn't nested transaction", !status.hasSavepoint()); + } + }); + + assertTrue("Hasn't thread connection", !TransactionSynchronizationManager.hasResource(ds)); + verify(con).rollback(sp); + verify(con).releaseSavepoint(sp); + verify(con).commit(); + verify(con).isReadOnly(); + verify(con).close(); + } + @Test public void testExistingTransactionWithManualSavepoint() throws Exception { DatabaseMetaData md = mock(DatabaseMetaData.class); diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java b/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java index 83b2631a3b5..37febe21dc6 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -677,12 +677,17 @@ public class JpaTransactionManager extends AbstractPlatformTransactionManager @Override public Object createSavepoint() throws TransactionException { + if (this.entityManagerHolder.isRollbackOnly()) { + throw new CannotCreateTransactionException( + "Cannot create savepoint for transaction which is already marked as rollback-only"); + } return getSavepointManager().createSavepoint(); } @Override public void rollbackToSavepoint(Object savepoint) throws TransactionException { getSavepointManager().rollbackToSavepoint(savepoint); + this.entityManagerHolder.resetRollbackOnly(); } @Override diff --git a/spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java b/spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java index 400863629bf..ce23664ba17 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java +++ b/spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java @@ -693,20 +693,15 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran if (defStatus.isDebug()) { logger.debug("Transactional code has requested rollback"); } - processRollback(defStatus); + processRollback(defStatus, false); return; } + if (!shouldCommitOnGlobalRollbackOnly() && defStatus.isGlobalRollbackOnly()) { if (defStatus.isDebug()) { logger.debug("Global transaction is marked as rollback-only but transactional code requested commit"); } - processRollback(defStatus); - // Throw UnexpectedRollbackException only at outermost transaction boundary - // or if explicitly asked to. - if (status.isNewTransaction() || isFailEarlyOnGlobalRollbackOnly()) { - throw new UnexpectedRollbackException( - "Transaction rolled back because it has been marked as rollback-only"); - } + processRollback(defStatus, true); return; } @@ -722,30 +717,35 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran private void processCommit(DefaultTransactionStatus status) throws TransactionException { try { boolean beforeCompletionInvoked = false; + try { + boolean unexpectedRollback = false; prepareForCommit(status); triggerBeforeCommit(status); triggerBeforeCompletion(status); beforeCompletionInvoked = true; - boolean globalRollbackOnly = false; - if (status.isNewTransaction() || isFailEarlyOnGlobalRollbackOnly()) { - globalRollbackOnly = status.isGlobalRollbackOnly(); - } + if (status.hasSavepoint()) { if (status.isDebug()) { logger.debug("Releasing transaction savepoint"); } + unexpectedRollback = status.isGlobalRollbackOnly(); status.releaseHeldSavepoint(); } else if (status.isNewTransaction()) { if (status.isDebug()) { logger.debug("Initiating transaction commit"); } + unexpectedRollback = status.isGlobalRollbackOnly(); doCommit(status); } + else if (isFailEarlyOnGlobalRollbackOnly()) { + unexpectedRollback = status.isGlobalRollbackOnly(); + } + // Throw UnexpectedRollbackException if we have a global rollback-only // marker but still didn't get a corresponding exception from commit. - if (globalRollbackOnly) { + if (unexpectedRollback) { throw new UnexpectedRollbackException( "Transaction silently rolled back because it has been marked as rollback-only"); } @@ -803,7 +803,7 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran } DefaultTransactionStatus defStatus = (DefaultTransactionStatus) status; - processRollback(defStatus); + processRollback(defStatus, false); } /** @@ -812,10 +812,13 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran * @param status object representing the transaction * @throws TransactionException in case of rollback failure */ - private void processRollback(DefaultTransactionStatus status) { + private void processRollback(DefaultTransactionStatus status, boolean unexpected) { try { + boolean unexpectedRollback = unexpected; + try { triggerBeforeCompletion(status); + if (status.hasSavepoint()) { if (status.isDebug()) { logger.debug("Rolling back transaction to savepoint"); @@ -828,28 +831,42 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran } doRollback(status); } - else if (status.hasTransaction()) { - if (status.isLocalRollbackOnly() || isGlobalRollbackOnParticipationFailure()) { - if (status.isDebug()) { - logger.debug("Participating transaction failed - marking existing transaction as rollback-only"); + else { + // Participating in larger transaction + if (status.hasTransaction()) { + if (status.isLocalRollbackOnly() || isGlobalRollbackOnParticipationFailure()) { + if (status.isDebug()) { + logger.debug("Participating transaction failed - marking existing transaction as rollback-only"); + } + doSetRollbackOnly(status); + } + else { + if (status.isDebug()) { + logger.debug("Participating transaction failed - letting transaction originator decide on rollback"); + } } - doSetRollbackOnly(status); } else { - if (status.isDebug()) { - logger.debug("Participating transaction failed - letting transaction originator decide on rollback"); - } + logger.debug("Should roll back transaction but cannot - no transaction available"); + } + // Unexpected rollback only matters here if we're asked to fail early + if (!isFailEarlyOnGlobalRollbackOnly()) { + unexpectedRollback = false; } - } - else { - logger.debug("Should roll back transaction but cannot - no transaction available"); } } catch (RuntimeException | Error ex) { triggerAfterCompletion(status, TransactionSynchronization.STATUS_UNKNOWN); throw ex; } + triggerAfterCompletion(status, TransactionSynchronization.STATUS_ROLLED_BACK); + + // Raise UnexpectedRollbackException if we had a global rollback-only marker + if (unexpectedRollback) { + throw new UnexpectedRollbackException( + "Transaction rolled back because it has been marked as rollback-only"); + } } finally { cleanupAfterCompletion(status); diff --git a/spring-tx/src/main/java/org/springframework/transaction/support/ResourceHolderSupport.java b/spring-tx/src/main/java/org/springframework/transaction/support/ResourceHolderSupport.java index bea1e3e864a..bf9ecb517f9 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/support/ResourceHolderSupport.java +++ b/spring-tx/src/main/java/org/springframework/transaction/support/ResourceHolderSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,9 +23,9 @@ import org.springframework.transaction.TransactionTimedOutException; /** * Convenient base class for resource holders. * - *
Features rollback-only support for nested transactions. - * Can expire after a certain number of seconds or milliseconds, - * to determine transactional timeouts. + *
Features rollback-only support for participating transactions. + * Can expire after a certain number of seconds or milliseconds + * in order to determine a transactional timeout. * * @author Juergen Hoeller * @since 02.02.2004 @@ -66,6 +66,17 @@ public abstract class ResourceHolderSupport implements ResourceHolder { this.rollbackOnly = true; } + /** + * Reset the rollback-only status for this resource transaction. + *
Only really intended to be called after custom rollback steps which + * keep the original resource in action, e.g. in case of a savepoint. + * @since 5.0 + * @see org.springframework.transaction.SavepointManager#rollbackToSavepoint + */ + public void resetRollbackOnly() { + this.rollbackOnly = false; + } + /** * Return whether the resource transaction is marked as rollback-only. */