From b5e9fa5f1e583e184f3165afaa5801b3b839fdbb Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 11 Feb 2019 11:45:43 +0100 Subject: [PATCH] TransactionActionSupport evaluates Vavr Try failure as rollback-only Closes gh-20361 --- spring-tx/spring-tx.gradle | 1 + .../interceptor/TransactionAspectSupport.java | 47 +++++- ...AnnotationTransactionInterceptorTests.java | 140 +++++++++++++++++- 3 files changed, 178 insertions(+), 10 deletions(-) diff --git a/spring-tx/spring-tx.gradle b/spring-tx/spring-tx.gradle index 093b9caad6..d293999edc 100644 --- a/spring-tx/spring-tx.gradle +++ b/spring-tx/spring-tx.gradle @@ -10,6 +10,7 @@ dependencies { optional("javax.resource:javax.resource-api:1.7.1") optional("javax.transaction:javax.transaction-api:1.3") optional("com.ibm.websphere:uow:6.0.2.17") + optional("io.vavr:vavr:0.10.0") testCompile("org.aspectj:aspectjweaver:${aspectjVersion}") testCompile("org.codehaus.groovy:groovy:${groovyVersion}") testCompile("org.eclipse.persistence:javax.persistence:2.2.0") 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 b1a8fa0495..cd0c3facb5 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -20,6 +20,7 @@ import java.lang.reflect.Method; import java.util.Properties; import java.util.concurrent.ConcurrentMap; +import io.vavr.control.Try; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -79,6 +80,12 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init */ private static final Object DEFAULT_TRANSACTION_MANAGER_KEY = new Object(); + /** + * Vavr library present on the classpath? + */ + private static final boolean vavrPresent = ClassUtils.isPresent( + "io.vavr.control.Try", TransactionAspectSupport.class.getClassLoader()); + /** * Holder to support the {@code currentTransactionStatus()} method, * and to support communication between different cooperating advices @@ -287,7 +294,8 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init if (txAttr == null || !(tm instanceof CallbackPreferringPlatformTransactionManager)) { // Standard transaction demarcation with getTransaction and commit/rollback calls. TransactionInfo txInfo = createTransactionIfNecessary(tm, txAttr, joinpointIdentification); - Object retVal = null; + + Object retVal; try { // This is an around advice: Invoke the next interceptor in the chain. // This will normally result in a target object being invoked. @@ -301,6 +309,15 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init finally { cleanupTransactionInfo(txInfo); } + + if (vavrPresent && VavrDelegate.isVavrTry(retVal)) { + // Set rollback-only in case of Vavr failure matching our rollback rules... + TransactionStatus status = txInfo.getTransactionStatus(); + if (status != null && txAttr != null) { + retVal = VavrDelegate.evaluateTryFailure(retVal, txAttr, status); + } + } + commitTransactionAfterReturning(txInfo); return retVal; } @@ -313,7 +330,12 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init Object result = ((CallbackPreferringPlatformTransactionManager) tm).execute(txAttr, status -> { TransactionInfo txInfo = prepareTransactionInfo(tm, txAttr, joinpointIdentification, status); try { - return invocation.proceedWithInvocation(); + Object retVal = invocation.proceedWithInvocation(); + if (vavrPresent && VavrDelegate.isVavrTry(retVal)) { + // Set rollback-only in case of Vavr failure matching our rollback rules... + retVal = VavrDelegate.evaluateTryFailure(retVal, txAttr, status); + } + return retVal; } catch (Throwable ex) { if (txAttr.rollbackOn(ex)) { @@ -712,4 +734,23 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init } } + + /** + * Inner class to avoid a hard dependency on the Vavr library at runtime. + */ + private static class VavrDelegate { + + public static boolean isVavrTry(Object retVal) { + return (retVal instanceof Try); + } + + public static Object evaluateTryFailure(Object retVal, TransactionAttribute txAttr, TransactionStatus status) { + return ((Try) retVal).onFailure(ex -> { + if (txAttr.rollbackOn(ex)) { + status.setRollbackOnly(); + } + }); + } + } + } diff --git a/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java b/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java index f35559342d..af0a6886ef 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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,6 +16,7 @@ package org.springframework.transaction.annotation; +import io.vavr.control.Try; import org.junit.Test; import org.springframework.aop.framework.ProxyFactory; @@ -123,12 +124,12 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withRollback() { + public void withRollbackOnRuntimeException() { ProxyFactory proxyFactory = new ProxyFactory(); - proxyFactory.setTarget(new TestWithRollback()); + proxyFactory.setTarget(new TestWithExceptions()); proxyFactory.addAdvice(this.ti); - TestWithRollback proxy = (TestWithRollback) proxyFactory.getProxy(); + TestWithExceptions proxy = (TestWithExceptions) proxyFactory.getProxy(); try { proxy.doSomethingErroneous(); @@ -147,6 +148,88 @@ public class AnnotationTransactionInterceptorTests { } } + @Test + public void withCommitOnCheckedException() { + ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(new TestWithExceptions()); + proxyFactory.addAdvice(this.ti); + + TestWithExceptions proxy = (TestWithExceptions) proxyFactory.getProxy(); + + try { + proxy.doSomethingElseWithCheckedException(); + fail("Should throw Exception"); + } + catch (Exception ex) { + assertGetTransactionAndCommitCount(1); + } + } + + @Test + public void withRollbackOnCheckedExceptionAndRollbackRule() { + ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(new TestWithExceptions()); + proxyFactory.addAdvice(this.ti); + + TestWithExceptions proxy = (TestWithExceptions) proxyFactory.getProxy(); + + try { + proxy.doSomethingElseWithCheckedExceptionAndRollbackRule(); + fail("Should throw Exception"); + } + catch (Exception ex) { + assertGetTransactionAndRollbackCount(1); + } + } + + @Test + public void withVavrTrySuccess() { + ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(new TestWithVavrTry()); + proxyFactory.addAdvice(this.ti); + + TestWithVavrTry proxy = (TestWithVavrTry) proxyFactory.getProxy(); + + proxy.doSomething(); + assertGetTransactionAndCommitCount(1); + } + + @Test + public void withVavrTryRuntimeException() { + ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(new TestWithVavrTry()); + proxyFactory.addAdvice(this.ti); + + TestWithVavrTry proxy = (TestWithVavrTry) proxyFactory.getProxy(); + + proxy.doSomethingErroneous(); + assertGetTransactionAndRollbackCount(1); + } + + @Test + public void withVavrTryCheckedException() { + ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(new TestWithVavrTry()); + proxyFactory.addAdvice(this.ti); + + TestWithVavrTry proxy = (TestWithVavrTry) proxyFactory.getProxy(); + + proxy.doSomethingErroneousWithCheckedException(); + assertGetTransactionAndCommitCount(1); + } + + @Test + public void withVavrTryCheckedExceptionAndRollbackRule() { + ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(new TestWithVavrTry()); + proxyFactory.addAdvice(this.ti); + + TestWithVavrTry proxy = (TestWithVavrTry) proxyFactory.getProxy(); + + proxy.doSomethingErroneousWithCheckedExceptionAndRollbackRule(); + assertGetTransactionAndRollbackCount(1); + } + @Test public void withInterface() { ProxyFactory proxyFactory = new ProxyFactory(); @@ -352,8 +435,8 @@ public class AnnotationTransactionInterceptorTests { } - @Transactional(rollbackFor = IllegalStateException.class) - public static class TestWithRollback { + @Transactional + public static class TestWithExceptions { public void doSomethingErroneous() { assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); @@ -361,12 +444,55 @@ public class AnnotationTransactionInterceptorTests { throw new IllegalStateException(); } - @Transactional(rollbackFor = IllegalArgumentException.class) public void doSomethingElseErroneous() { assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly()); throw new IllegalArgumentException(); } + + @Transactional + public void doSomethingElseWithCheckedException() throws Exception { + assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); + assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly()); + throw new Exception(); + } + + @Transactional(rollbackFor = Exception.class) + public void doSomethingElseWithCheckedExceptionAndRollbackRule() throws Exception { + assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); + assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly()); + throw new Exception(); + } + } + + + @Transactional + public static class TestWithVavrTry { + + public Try doSomething() { + assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); + assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly()); + return Try.success("ok"); + } + + public Try doSomethingErroneous() { + assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); + assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly()); + return Try.failure(new IllegalStateException()); + } + + public Try doSomethingErroneousWithCheckedException() { + assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); + assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly()); + return Try.failure(new Exception()); + } + + @Transactional(rollbackFor = Exception.class) + public Try doSomethingErroneousWithCheckedExceptionAndRollbackRule() { + assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); + assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly()); + return Try.failure(new Exception()); + } }