diff --git a/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AbstractTransactionAspect.aj b/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AbstractTransactionAspect.aj index 54bd4ef7464..ec5e803a396 100644 --- a/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AbstractTransactionAspect.aj +++ b/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AbstractTransactionAspect.aj @@ -16,8 +16,6 @@ package org.springframework.transaction.aspectj; -import java.lang.reflect.Method; - import org.aspectj.lang.annotation.SuppressAjWarnings; import org.aspectj.lang.reflect.MethodSignature; import org.springframework.transaction.interceptor.TransactionAspectSupport; @@ -42,45 +40,42 @@ import org.springframework.transaction.interceptor.TransactionAttributeSource; * * @author Rod Johnson * @author Ramnivas Laddad + * @author Juergen Hoeller * @since 2.0 */ public abstract aspect AbstractTransactionAspect extends TransactionAspectSupport { /** - * Construct object using the given transaction metadata retrieval strategy. + * Construct the aspect using the given transaction metadata retrieval strategy. * @param tas TransactionAttributeSource implementation, retrieving Spring - * transaction metadata for each joinpoint. Write the subclass to pass in null - * if it's intended to be configured by Setter Injection. + * transaction metadata for each joinpoint. Implement the subclass to pass in + * {@code null} if it is intended to be configured through Setter Injection. */ protected AbstractTransactionAspect(TransactionAttributeSource tas) { setTransactionAttributeSource(tas); } @SuppressAjWarnings("adviceDidNotMatch") - before(Object txObject) : transactionalMethodExecution(txObject) { + Object around(final Object txObject): transactionalMethodExecution(txObject) { MethodSignature methodSignature = (MethodSignature) thisJoinPoint.getSignature(); - Method method = methodSignature.getMethod(); - createTransactionIfNecessary(method, txObject.getClass()); - } - - @SuppressAjWarnings("adviceDidNotMatch") - after(Object txObject) throwing(Throwable t) : transactionalMethodExecution(txObject) { + // Adapt to TransactionAspectSupport's invokeWithinTransaction... try { - completeTransactionAfterThrowing(TransactionAspectSupport.currentTransactionInfo(), t); + return invokeWithinTransaction(methodSignature.getMethod(), txObject.getClass(), new InvocationCallback() { + public Object proceedWithInvocation() throws Throwable { + return proceed(txObject); + } + }); } - catch (Throwable t2) { - logger.error("Failed to close transaction after throwing in a transactional method", t2); + catch (RuntimeException ex) { + throw ex; + } + catch (Error err) { + throw err; + } + catch (Throwable thr) { + Rethrower.rethrow(thr); + throw new IllegalStateException("Should never get here", thr); } - } - - @SuppressAjWarnings("adviceDidNotMatch") - after(Object txObject) returning() : transactionalMethodExecution(txObject) { - commitTransactionAfterReturning(TransactionAspectSupport.currentTransactionInfo()); - } - - @SuppressAjWarnings("adviceDidNotMatch") - after(Object txObject) : transactionalMethodExecution(txObject) { - cleanupTransactionInfo(TransactionAspectSupport.currentTransactionInfo()); } /** @@ -90,4 +85,22 @@ public abstract aspect AbstractTransactionAspect extends TransactionAspectSuppor */ protected abstract pointcut transactionalMethodExecution(Object txObject); + + /** + * Ugly but safe workaround: We need to be able to propagate checked exceptions, + * despite AspectJ around advice supporting specifically declared exceptions only. + */ + private static class Rethrower { + + public static void rethrow(final Throwable exception) { + class CheckedExceptionRethrower { + @SuppressWarnings("unchecked") + private void rethrow(Throwable exception) throws T { + throw (T) exception; + } + } + new CheckedExceptionRethrower().rethrow(exception); + } + } + } diff --git a/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AnnotationTransactionAspect.aj b/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AnnotationTransactionAspect.aj index 2ea8f9e3f58..026704ac087 100644 --- a/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AnnotationTransactionAspect.aj +++ b/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AnnotationTransactionAspect.aj @@ -49,16 +49,14 @@ public aspect AnnotationTransactionAspect extends AbstractTransactionAspect { } /** - * Matches the execution of any public method in a type with the - * Transactional annotation, or any subtype of a type with the - * Transactional annotation. + * Matches the execution of any public method in a type with the Transactional + * annotation, or any subtype of a type with the Transactional annotation. */ private pointcut executionOfAnyPublicMethodInAtTransactionalType() : execution(public * ((@Transactional *)+).*(..)) && within(@Transactional *); /** - * Matches the execution of any method with the - * Transactional annotation. + * Matches the execution of any method with the Transactional annotation. */ private pointcut executionOfTransactionalMethod() : execution(@Transactional * *(..)); diff --git a/spring-aspects/src/test/java/org/springframework/transaction/aspectj/TransactionAspectTests.java b/spring-aspects/src/test/java/org/springframework/transaction/aspectj/TransactionAspectTests.java index 70892e39493..b69e861933a 100644 --- a/spring-aspects/src/test/java/org/springframework/transaction/aspectj/TransactionAspectTests.java +++ b/spring-aspects/src/test/java/org/springframework/transaction/aspectj/TransactionAspectTests.java @@ -16,19 +16,18 @@ package org.springframework.transaction.aspectj; -import java.lang.reflect.Method; - -import junit.framework.AssertionFailedError; - import org.springframework.test.AbstractDependencyInjectionSpringContextTests; import org.springframework.tests.transaction.CallCountingTransactionManager; import org.springframework.transaction.annotation.AnnotationTransactionAttributeSource; import org.springframework.transaction.interceptor.TransactionAspectSupport; import org.springframework.transaction.interceptor.TransactionAttribute; +import java.lang.reflect.Method; + /** * @author Rod Johnson * @author Ramnivas Laddad + * @author Juergen Hoeller */ public class TransactionAspectTests extends AbstractDependencyInjectionSpringContextTests { @@ -123,50 +122,63 @@ public class TransactionAspectTests extends AbstractDependencyInjectionSpringCon public void testDefaultCommitOnAnnotatedClass() throws Throwable { - testRollback(new TransactionOperationCallback() { - public Object performTransactionalOperation() throws Throwable { - return annotationOnlyOnClassWithNoInterface.echo(new Exception()); - } - }, false); + final Exception ex = new Exception(); + try { + testRollback(new TransactionOperationCallback() { + public Object performTransactionalOperation() throws Throwable { + return annotationOnlyOnClassWithNoInterface.echo(ex); + } + }, false); + fail("Should have thrown Exception"); + } + catch (Exception ex2) { + assertSame(ex, ex2); + } } public void testDefaultRollbackOnAnnotatedClass() throws Throwable { - testRollback(new TransactionOperationCallback() { - public Object performTransactionalOperation() throws Throwable { - return annotationOnlyOnClassWithNoInterface.echo(new RuntimeException()); - } - }, true); + final RuntimeException ex = new RuntimeException(); + try { + testRollback(new TransactionOperationCallback() { + public Object performTransactionalOperation() throws Throwable { + return annotationOnlyOnClassWithNoInterface.echo(ex); + } + }, true); + fail("Should have thrown RuntimeException"); + } + catch (RuntimeException ex2) { + assertSame(ex, ex2); + } } - public static class SubclassOfClassWithTransactionalAnnotation extends TransactionalAnnotationOnlyOnClassWithNoInterface { - } - public void testDefaultCommitOnSubclassOfAnnotatedClass() throws Throwable { - testRollback(new TransactionOperationCallback() { - public Object performTransactionalOperation() throws Throwable { - return new SubclassOfClassWithTransactionalAnnotation().echo(new Exception()); - } - }, false); - } - - public static class SubclassOfClassWithTransactionalMethodAnnotation extends MethodAnnotationOnClassWithNoInterface { + final Exception ex = new Exception(); + try { + testRollback(new TransactionOperationCallback() { + public Object performTransactionalOperation() throws Throwable { + return new SubclassOfClassWithTransactionalAnnotation().echo(ex); + } + }, false); + fail("Should have thrown Exception"); + } + catch (Exception ex2) { + assertSame(ex, ex2); + } } public void testDefaultCommitOnSubclassOfClassWithTransactionalMethodAnnotated() throws Throwable { - testRollback(new TransactionOperationCallback() { - public Object performTransactionalOperation() throws Throwable { - return new SubclassOfClassWithTransactionalMethodAnnotation().echo(new Exception()); - } - }, false); - } - - public static class ImplementsAnnotatedInterface implements ITransactional { - public Object echo(Throwable t) throws Throwable { - if (t != null) { - throw t; - } - return t; + final Exception ex = new Exception(); + try { + testRollback(new TransactionOperationCallback() { + public Object performTransactionalOperation() throws Throwable { + return new SubclassOfClassWithTransactionalMethodAnnotation().echo(ex); + } + }, false); + fail("Should have thrown Exception"); + } + catch (Exception ex2) { + assertSame(ex, ex2); } } @@ -221,18 +233,12 @@ public class TransactionAspectTests extends AbstractDependencyInjectionSpringCon assertEquals(0, txManager.begun); try { toc.performTransactionalOperation(); - assertEquals(1, txManager.commits); } - catch (Throwable caught) { - if (caught instanceof AssertionFailedError) { - return; - } + finally { + assertEquals(1, txManager.begun); + assertEquals(rollback ? 0 : 1, txManager.commits); + assertEquals(rollback ? 1 : 0, txManager.rollbacks); } - - if (rollback) { - assertEquals(1, txManager.rollbacks); - } - assertEquals(1, txManager.begun); } protected void testNotTransactional(TransactionOperationCallback toc, Throwable expected) throws Throwable { @@ -258,4 +264,23 @@ public class TransactionAspectTests extends AbstractDependencyInjectionSpringCon Object performTransactionalOperation() throws Throwable; } + + public static class SubclassOfClassWithTransactionalAnnotation extends TransactionalAnnotationOnlyOnClassWithNoInterface { + } + + + public static class SubclassOfClassWithTransactionalMethodAnnotation extends MethodAnnotationOnClassWithNoInterface { + } + + + public static class ImplementsAnnotatedInterface implements ITransactional { + + public Object echo(Throwable t) throws Throwable { + if (t != null) { + throw t; + } + return t; + } + } + } diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java index 9f9b9503ddf..bcd2c778805 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java @@ -16,12 +16,8 @@ package org.springframework.transaction.interceptor; -import java.lang.reflect.Method; -import java.util.Properties; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.InitializingBean; @@ -31,8 +27,13 @@ import org.springframework.transaction.NoTransactionException; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.TransactionSystemException; +import org.springframework.transaction.support.CallbackPreferringPlatformTransactionManager; +import org.springframework.transaction.support.TransactionCallback; import org.springframework.util.StringUtils; +import java.lang.reflect.Method; +import java.util.Properties; + /** * Base class for transactional aspects, such as the {@link TransactionInterceptor} * or an AspectJ aspect. @@ -231,6 +232,90 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init } + /** + * General delegate for around-advice-based subclasses, delegating to several other template + * methods on this class. Able to handle {@link CallbackPreferringPlatformTransactionManager} + * as well as regular {@link PlatformTransactionManager} implementations. + * @param method the Method being invoked + * @param targetClass the target class that we're invoking the method on + * @param invocation the callback to use for proceeding with the target invocation + * @return the return value of the method, if any + * @throws Throwable propagated from the target invocation + */ + protected Object invokeWithinTransaction(Method method, Class targetClass, final InvocationCallback invocation) + throws Throwable { + + // If the transaction attribute is null, the method is non-transactional. + final TransactionAttribute txAttr = getTransactionAttributeSource().getTransactionAttribute(method, targetClass); + final PlatformTransactionManager tm = determineTransactionManager(txAttr); + final String joinpointIdentification = methodIdentification(method, targetClass); + + if (txAttr == null || !(tm instanceof CallbackPreferringPlatformTransactionManager)) { + // Standard transaction demarcation with getTransaction and commit/rollback calls. + TransactionInfo txInfo = createTransactionIfNecessary(tm, txAttr, joinpointIdentification); + Object retVal = null; + try { + // This is an around advice: Invoke the next interceptor in the chain. + // This will normally result in a target object being invoked. + retVal = invocation.proceedWithInvocation(); + } + catch (Throwable ex) { + // target invocation exception + completeTransactionAfterThrowing(txInfo, ex); + throw ex; + } + finally { + cleanupTransactionInfo(txInfo); + } + commitTransactionAfterReturning(txInfo); + return retVal; + } + + else { + // It's a CallbackPreferringPlatformTransactionManager: pass a TransactionCallback in. + try { + Object result = ((CallbackPreferringPlatformTransactionManager) tm).execute(txAttr, + new TransactionCallback() { + public Object doInTransaction(TransactionStatus status) { + TransactionInfo txInfo = prepareTransactionInfo(tm, txAttr, joinpointIdentification, status); + try { + return invocation.proceedWithInvocation(); + } + catch (Throwable ex) { + if (txAttr.rollbackOn(ex)) { + // A RuntimeException: will lead to a rollback. + if (ex instanceof RuntimeException) { + throw (RuntimeException) ex; + } + else { + throw new ThrowableHolderException(ex); + } + } + else { + // A normal return value: will lead to a commit. + return new ThrowableHolder(ex); + } + } + finally { + cleanupTransactionInfo(txInfo); + } + } + }); + + // Check result: It might indicate a Throwable to rethrow. + if (result instanceof ThrowableHolder) { + throw ((ThrowableHolder) result).getThrowable(); + } + else { + return result; + } + } + catch (ThrowableHolderException ex) { + throw ex.getCause(); + } + } + } + /** * Determine the specific transaction manager to use for the given transaction. */ @@ -250,23 +335,6 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init } } - /** - * Create a transaction if necessary, based on the given method and class. - *

Performs a default TransactionAttribute lookup for the given method. - * @param method the method about to execute - * @param targetClass the class that the method is being invoked on - * @return a TransactionInfo object, whether or not a transaction was created. - * The {@code hasTransaction()} method on TransactionInfo can be used to - * tell if there was a transaction created. - * @see #getTransactionAttributeSource() - */ - protected TransactionInfo createTransactionIfNecessary(Method method, Class targetClass) { - // If the transaction attribute is null, the method is non-transactional. - TransactionAttribute txAttr = getTransactionAttributeSource().getTransactionAttribute(method, targetClass); - PlatformTransactionManager tm = determineTransactionManager(txAttr); - return createTransactionIfNecessary(tm, txAttr, methodIdentification(method, targetClass)); - } - /** * Convenience method to return a String representation of this Method * for use in logging. Can be overridden in subclasses to provide a @@ -297,6 +365,26 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init return null; } + /** + * Create a transaction if necessary, based on the given method and class. + *

Performs a default TransactionAttribute lookup for the given method. + * @param method the method about to execute + * @param targetClass the class that the method is being invoked on + * @return a TransactionInfo object, whether or not a transaction was created. + * The {@code hasTransaction()} method on TransactionInfo can be used to + * tell if there was a transaction created. + * @see #getTransactionAttributeSource() + * @deprecated in favor of + * {@link #createTransactionIfNecessary(PlatformTransactionManager, TransactionAttribute, String)} + */ + @Deprecated + protected TransactionInfo createTransactionIfNecessary(Method method, Class targetClass) { + // If the transaction attribute is null, the method is non-transactional. + TransactionAttribute txAttr = getTransactionAttributeSource().getTransactionAttribute(method, targetClass); + PlatformTransactionManager tm = determineTransactionManager(txAttr); + return createTransactionIfNecessary(tm, txAttr, methodIdentification(method, targetClass)); + } + /** * Create a transaction if necessary based on the given TransactionAttribute. *

Allows callers to perform custom TransactionAttribute lookups through @@ -527,4 +615,50 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init } } + + /** + * Simple callback interface for proceeding with the target invocation. + * Concrete interceptors/aspects adapt this to their invocation mechanism. + */ + protected interface InvocationCallback { + + Object proceedWithInvocation() throws Throwable; + } + + + /** + * Internal holder class for a Throwable, used as a return value + * from a TransactionCallback (to be subsequently unwrapped again). + */ + private static class ThrowableHolder { + + private final Throwable throwable; + + public ThrowableHolder(Throwable throwable) { + this.throwable = throwable; + } + + public final Throwable getThrowable() { + return this.throwable; + } + } + + + /** + * Internal holder class for a Throwable, used as a RuntimeException to be + * thrown from a TransactionCallback (and subsequently unwrapped again). + */ + @SuppressWarnings("serial") + private static class ThrowableHolderException extends RuntimeException { + + public ThrowableHolderException(Throwable throwable) { + super(throwable); + } + + @Override + public String toString() { + return getCause().toString(); + } + } + } diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionInterceptor.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionInterceptor.java index 11f72759a7f..db46387077c 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionInterceptor.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -16,22 +16,18 @@ package org.springframework.transaction.interceptor; +import org.aopalliance.intercept.MethodInterceptor; +import org.aopalliance.intercept.MethodInvocation; +import org.springframework.aop.support.AopUtils; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.transaction.PlatformTransactionManager; + import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; import java.util.Properties; -import org.aopalliance.intercept.MethodInterceptor; -import org.aopalliance.intercept.MethodInvocation; - -import org.springframework.aop.support.AopUtils; -import org.springframework.beans.factory.BeanFactory; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.TransactionStatus; -import org.springframework.transaction.support.CallbackPreferringPlatformTransactionManager; -import org.springframework.transaction.support.TransactionCallback; - /** * AOP Alliance MethodInterceptor for declarative transaction * management using the common Spring transaction infrastructure @@ -40,7 +36,7 @@ import org.springframework.transaction.support.TransactionCallback; *

Derives from the {@link TransactionAspectSupport} class which * contains the integration with Spring's underlying transaction API. * TransactionInterceptor simply calls the relevant superclass methods - * such as {@link #createTransactionIfNecessary} in the correct order. + * such as {@link #invokeWithinTransaction} in the correct order. * *

TransactionInterceptors are thread-safe. * @@ -94,76 +90,12 @@ public class TransactionInterceptor extends TransactionAspectSupport implements // as well as the method, which may be from an interface. Class targetClass = (invocation.getThis() != null ? AopUtils.getTargetClass(invocation.getThis()) : null); - // If the transaction attribute is null, the method is non-transactional. - final TransactionAttribute txAttr = - getTransactionAttributeSource().getTransactionAttribute(invocation.getMethod(), targetClass); - final PlatformTransactionManager tm = determineTransactionManager(txAttr); - final String joinpointIdentification = methodIdentification(invocation.getMethod(), targetClass); - - if (txAttr == null || !(tm instanceof CallbackPreferringPlatformTransactionManager)) { - // Standard transaction demarcation with getTransaction and commit/rollback calls. - TransactionInfo txInfo = createTransactionIfNecessary(tm, txAttr, joinpointIdentification); - Object retVal = null; - try { - // This is an around advice: Invoke the next interceptor in the chain. - // This will normally result in a target object being invoked. - retVal = invocation.proceed(); + // Adapt to TransactionAspectSupport's invokeWithinTransaction... + return invokeWithinTransaction(invocation.getMethod(), targetClass, new InvocationCallback() { + public Object proceedWithInvocation() throws Throwable { + return invocation.proceed(); } - catch (Throwable ex) { - // target invocation exception - completeTransactionAfterThrowing(txInfo, ex); - throw ex; - } - finally { - cleanupTransactionInfo(txInfo); - } - commitTransactionAfterReturning(txInfo); - return retVal; - } - - else { - // It's a CallbackPreferringPlatformTransactionManager: pass a TransactionCallback in. - try { - Object result = ((CallbackPreferringPlatformTransactionManager) tm).execute(txAttr, - new TransactionCallback() { - public Object doInTransaction(TransactionStatus status) { - TransactionInfo txInfo = prepareTransactionInfo(tm, txAttr, joinpointIdentification, status); - try { - return invocation.proceed(); - } - catch (Throwable ex) { - if (txAttr.rollbackOn(ex)) { - // A RuntimeException: will lead to a rollback. - if (ex instanceof RuntimeException) { - throw (RuntimeException) ex; - } - else { - throw new ThrowableHolderException(ex); - } - } - else { - // A normal return value: will lead to a commit. - return new ThrowableHolder(ex); - } - } - finally { - cleanupTransactionInfo(txInfo); - } - } - }); - - // Check result: It might indicate a Throwable to rethrow. - if (result instanceof ThrowableHolder) { - throw ((ThrowableHolder) result).getThrowable(); - } - else { - return result; - } - } - catch (ThrowableHolderException ex) { - throw ex.getCause(); - } - } + }); } @@ -195,39 +127,4 @@ public class TransactionInterceptor extends TransactionAspectSupport implements setBeanFactory((BeanFactory) ois.readObject()); } - - /** - * Internal holder class for a Throwable, used as a return value - * from a TransactionCallback (to be subsequently unwrapped again). - */ - private static class ThrowableHolder { - - private final Throwable throwable; - - public ThrowableHolder(Throwable throwable) { - this.throwable = throwable; - } - - public final Throwable getThrowable() { - return this.throwable; - } - } - - - /** - * Internal holder class for a Throwable, used as a RuntimeException to be - * thrown from a TransactionCallback (and subsequently unwrapped again). - */ - private static class ThrowableHolderException extends RuntimeException { - - public ThrowableHolderException(Throwable throwable) { - super(throwable); - } - - @Override - public String toString() { - return getCause().toString(); - } - } - }