TransactionActionSupport evaluates Vavr Try failure as rollback-only

Closes gh-20361

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")

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();
+				}
+			});
+		}
+	}
+
 }

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)
+	@Transactional
-	public static class TestWithRollback {
+	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<String> doSomething() {
+			assertTrue(TransactionSynchronizationManager.isActualTransactionActive());
+			assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly());
+			return Try.success("ok");
+		}
+
+		public Try<String> doSomethingErroneous() {
+			assertTrue(TransactionSynchronizationManager.isActualTransactionActive());
+			assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly());
+			return Try.failure(new IllegalStateException());
+		}
+
+		public Try<String> doSomethingErroneousWithCheckedException() {
+			assertTrue(TransactionSynchronizationManager.isActualTransactionActive());
+			assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly());
+			return Try.failure(new Exception());
+		}
+
+		@Transactional(rollbackFor = Exception.class)
+		public Try<String> doSomethingErroneousWithCheckedExceptionAndRollbackRule() {
+			assertTrue(TransactionSynchronizationManager.isActualTransactionActive());
+			assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly());
+			return Try.failure(new Exception());
+		}
 	}