Skip to content

Commit dd871e0

Browse files
jhoellerjhoeller
committed
Trigger rollback for (Completable)Future exception on method return
Closes gh-30018
1 parent c1014f5 commit dd871e0

File tree

2 files changed

+99
-27
lines changed

2 files changed

+99
-27
lines changed

‎spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.lang.reflect.Method;
2020
import java.util.Properties;
2121
import java.util.concurrent.ConcurrentMap;
22+
import java.util.concurrent.ExecutionException;
23+
import java.util.concurrent.Future;
2224

2325
import io.vavr.control.Try;
2426
import kotlin.coroutines.Continuation;
@@ -399,11 +401,26 @@ protected Object invokeWithinTransaction(Method method, @Nullable Class<?> targe
399401
cleanupTransactionInfo(txInfo);
400402
}
401403

402-
if (retVal != null && vavrPresent && VavrDelegate.isVavrTry(retVal)) {
403-
// Set rollback-only in case of Vavr failure matching our rollback rules...
404+
if (retVal != null && txAttr != null) {
404405
TransactionStatus status = txInfo.getTransactionStatus();
405-
if (status != null && txAttr != null) {
406-
retVal = VavrDelegate.evaluateTryFailure(retVal, txAttr, status);
406+
if (status != null) {
407+
if (retVal instanceof Future<?> future && future.isDone()) {
408+
try {
409+
future.get();
410+
}
411+
catch (ExecutionException ex) {
412+
if (txAttr.rollbackOn(ex.getCause())) {
413+
status.setRollbackOnly();
414+
}
415+
}
416+
catch (InterruptedException ex) {
417+
Thread.currentThread().interrupt();
418+
}
419+
}
420+
else if (vavrPresent && VavrDelegate.isVavrTry(retVal)) {
421+
// Set rollback-only in case of Vavr failure matching our rollback rules...
422+
retVal = VavrDelegate.evaluateTryFailure(retVal, txAttr, status);
423+
}
407424
}
408425
}
409426

‎spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java

Lines changed: 78 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
1717
package org.springframework.transaction.annotation;
1818

1919
import java.time.Duration;
20+
import java.util.concurrent.CompletableFuture;
2021

2122
import io.vavr.control.Try;
2223
import org.junit.jupiter.api.Test;
@@ -57,7 +58,6 @@ public void classLevelOnly() {
5758
ProxyFactory proxyFactory = new ProxyFactory();
5859
proxyFactory.setTarget(new TestClassLevelOnly());
5960
proxyFactory.addAdvice(this.ti);
60-
6161
TestClassLevelOnly proxy = (TestClassLevelOnly) proxyFactory.getProxy();
6262

6363
proxy.doSomething();
@@ -78,7 +78,6 @@ public void withSingleMethodOverride() {
7878
ProxyFactory proxyFactory = new ProxyFactory();
7979
proxyFactory.setTarget(new TestWithSingleMethodOverride());
8080
proxyFactory.addAdvice(this.ti);
81-
8281
TestWithSingleMethodOverride proxy = (TestWithSingleMethodOverride) proxyFactory.getProxy();
8382

8483
proxy.doSomething();
@@ -99,7 +98,6 @@ public void withSingleMethodOverrideInverted() {
9998
ProxyFactory proxyFactory = new ProxyFactory();
10099
proxyFactory.setTarget(new TestWithSingleMethodOverrideInverted());
101100
proxyFactory.addAdvice(this.ti);
102-
103101
TestWithSingleMethodOverrideInverted proxy = (TestWithSingleMethodOverrideInverted) proxyFactory.getProxy();
104102

105103
proxy.doSomething();
@@ -120,7 +118,6 @@ public void withMultiMethodOverride() {
120118
ProxyFactory proxyFactory = new ProxyFactory();
121119
proxyFactory.setTarget(new TestWithMultiMethodOverride());
122120
proxyFactory.addAdvice(this.ti);
123-
124121
TestWithMultiMethodOverride proxy = (TestWithMultiMethodOverride) proxyFactory.getProxy();
125122

126123
proxy.doSomething();
@@ -141,7 +138,6 @@ public void withRollbackOnRuntimeException() {
141138
ProxyFactory proxyFactory = new ProxyFactory();
142139
proxyFactory.setTarget(new TestWithExceptions());
143140
proxyFactory.addAdvice(this.ti);
144-
145141
TestWithExceptions proxy = (TestWithExceptions) proxyFactory.getProxy();
146142

147143
assertThatIllegalStateException().isThrownBy(
@@ -158,7 +154,6 @@ public void withCommitOnCheckedException() {
158154
ProxyFactory proxyFactory = new ProxyFactory();
159155
proxyFactory.setTarget(new TestWithExceptions());
160156
proxyFactory.addAdvice(this.ti);
161-
162157
TestWithExceptions proxy = (TestWithExceptions) proxyFactory.getProxy();
163158

164159
assertThatException()
@@ -171,7 +166,6 @@ public void withRollbackOnCheckedExceptionAndRollbackRule() {
171166
ProxyFactory proxyFactory = new ProxyFactory();
172167
proxyFactory.setTarget(new TestWithExceptions());
173168
proxyFactory.addAdvice(this.ti);
174-
175169
TestWithExceptions proxy = (TestWithExceptions) proxyFactory.getProxy();
176170

177171
assertThatException()
@@ -184,7 +178,6 @@ public void withMonoSuccess() {
184178
ProxyFactory proxyFactory = new ProxyFactory();
185179
proxyFactory.setTarget(new TestWithReactive());
186180
proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source));
187-
188181
TestWithReactive proxy = (TestWithReactive) proxyFactory.getProxy();
189182

190183
StepVerifier.withVirtualTime(proxy::monoSuccess).thenAwait(Duration.ofSeconds(10)).verifyComplete();
@@ -196,7 +189,6 @@ public void withMonoFailure() {
196189
ProxyFactory proxyFactory = new ProxyFactory();
197190
proxyFactory.setTarget(new TestWithReactive());
198191
proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source));
199-
200192
TestWithReactive proxy = (TestWithReactive) proxyFactory.getProxy();
201193

202194
proxy.monoFailure().as(StepVerifier::create).verifyError();
@@ -208,7 +200,6 @@ public void withMonoRollback() {
208200
ProxyFactory proxyFactory = new ProxyFactory();
209201
proxyFactory.setTarget(new TestWithReactive());
210202
proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source));
211-
212203
TestWithReactive proxy = (TestWithReactive) proxyFactory.getProxy();
213204

214205
StepVerifier.withVirtualTime(proxy::monoSuccess).thenAwait(Duration.ofSeconds(1)).thenCancel().verify();
@@ -220,7 +211,6 @@ public void withFluxSuccess() {
220211
ProxyFactory proxyFactory = new ProxyFactory();
221212
proxyFactory.setTarget(new TestWithReactive());
222213
proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source));
223-
224214
TestWithReactive proxy = (TestWithReactive) proxyFactory.getProxy();
225215

226216
StepVerifier.withVirtualTime(proxy::fluxSuccess).thenAwait(Duration.ofSeconds(10)).expectNextCount(1).verifyComplete();
@@ -232,7 +222,6 @@ public void withFluxFailure() {
232222
ProxyFactory proxyFactory = new ProxyFactory();
233223
proxyFactory.setTarget(new TestWithReactive());
234224
proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source));
235-
236225
TestWithReactive proxy = (TestWithReactive) proxyFactory.getProxy();
237226

238227
proxy.fluxFailure().as(StepVerifier::create).verifyError();
@@ -244,19 +233,61 @@ public void withFluxRollback() {
244233
ProxyFactory proxyFactory = new ProxyFactory();
245234
proxyFactory.setTarget(new TestWithReactive());
246235
proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source));
247-
248236
TestWithReactive proxy = (TestWithReactive) proxyFactory.getProxy();
249237

250238
StepVerifier.withVirtualTime(proxy::fluxSuccess).thenAwait(Duration.ofSeconds(1)).thenCancel().verify();
251239
assertReactiveGetTransactionAndRollbackCount(1);
252240
}
253241

242+
@Test
243+
public void withCompletableFutureSuccess() {
244+
ProxyFactory proxyFactory = new ProxyFactory();
245+
proxyFactory.setTarget(new TestWithCompletableFuture());
246+
proxyFactory.addAdvice(this.ti);
247+
TestWithCompletableFuture proxy = (TestWithCompletableFuture) proxyFactory.getProxy();
248+
249+
proxy.doSomething();
250+
assertGetTransactionAndCommitCount(1);
251+
}
252+
253+
@Test
254+
public void withCompletableFutureRuntimeException() {
255+
ProxyFactory proxyFactory = new ProxyFactory();
256+
proxyFactory.setTarget(new TestWithCompletableFuture());
257+
proxyFactory.addAdvice(this.ti);
258+
TestWithCompletableFuture proxy = (TestWithCompletableFuture) proxyFactory.getProxy();
259+
260+
proxy.doSomethingErroneous();
261+
assertGetTransactionAndRollbackCount(1);
262+
}
263+
264+
@Test
265+
public void withCompletableFutureCheckedException() {
266+
ProxyFactory proxyFactory = new ProxyFactory();
267+
proxyFactory.setTarget(new TestWithCompletableFuture());
268+
proxyFactory.addAdvice(this.ti);
269+
TestWithCompletableFuture proxy = (TestWithCompletableFuture) proxyFactory.getProxy();
270+
271+
proxy.doSomethingErroneousWithCheckedException();
272+
assertGetTransactionAndCommitCount(1);
273+
}
274+
275+
@Test
276+
public void withCompletableFutureCheckedExceptionAndRollbackRule() {
277+
ProxyFactory proxyFactory = new ProxyFactory();
278+
proxyFactory.setTarget(new TestWithCompletableFuture());
279+
proxyFactory.addAdvice(this.ti);
280+
TestWithCompletableFuture proxy = (TestWithCompletableFuture) proxyFactory.getProxy();
281+
282+
proxy.doSomethingErroneousWithCheckedExceptionAndRollbackRule();
283+
assertGetTransactionAndRollbackCount(1);
284+
}
285+
254286
@Test
255287
public void withVavrTrySuccess() {
256288
ProxyFactory proxyFactory = new ProxyFactory();
257289
proxyFactory.setTarget(new TestWithVavrTry());
258290
proxyFactory.addAdvice(this.ti);
259-
260291
TestWithVavrTry proxy = (TestWithVavrTry) proxyFactory.getProxy();
261292

262293
proxy.doSomething();
@@ -268,7 +299,6 @@ public void withVavrTryRuntimeException() {
268299
ProxyFactory proxyFactory = new ProxyFactory();
269300
proxyFactory.setTarget(new TestWithVavrTry());
270301
proxyFactory.addAdvice(this.ti);
271-
272302
TestWithVavrTry proxy = (TestWithVavrTry) proxyFactory.getProxy();
273303

274304
proxy.doSomethingErroneous();
@@ -280,7 +310,6 @@ public void withVavrTryCheckedException() {
280310
ProxyFactory proxyFactory = new ProxyFactory();
281311
proxyFactory.setTarget(new TestWithVavrTry());
282312
proxyFactory.addAdvice(this.ti);
283-
284313
TestWithVavrTry proxy = (TestWithVavrTry) proxyFactory.getProxy();
285314

286315
proxy.doSomethingErroneousWithCheckedException();
@@ -292,7 +321,6 @@ public void withVavrTryCheckedExceptionAndRollbackRule() {
292321
ProxyFactory proxyFactory = new ProxyFactory();
293322
proxyFactory.setTarget(new TestWithVavrTry());
294323
proxyFactory.addAdvice(this.ti);
295-
296324
TestWithVavrTry proxy = (TestWithVavrTry) proxyFactory.getProxy();
297325

298326
proxy.doSomethingErroneousWithCheckedExceptionAndRollbackRule();
@@ -305,7 +333,6 @@ public void withInterface() {
305333
proxyFactory.setTarget(new TestWithInterfaceImpl());
306334
proxyFactory.addInterface(TestWithInterface.class);
307335
proxyFactory.addAdvice(this.ti);
308-
309336
TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy();
310337

311338
proxy.doSomething();
@@ -330,7 +357,6 @@ public void crossClassInterfaceMethodLevelOnJdkProxy() {
330357
proxyFactory.setTarget(new SomeServiceImpl());
331358
proxyFactory.addInterface(SomeService.class);
332359
proxyFactory.addAdvice(this.ti);
333-
334360
SomeService someService = (SomeService) proxyFactory.getProxy();
335361

336362
someService.bar();
@@ -349,7 +375,6 @@ public void crossClassInterfaceOnJdkProxy() {
349375
proxyFactory.setTarget(new OtherServiceImpl());
350376
proxyFactory.addInterface(OtherService.class);
351377
proxyFactory.addAdvice(this.ti);
352-
353378
OtherService otherService = (OtherService) proxyFactory.getProxy();
354379

355380
otherService.foo();
@@ -366,7 +391,6 @@ public void withInterfaceOnTargetJdkProxy() {
366391
proxyFactory.setTarget(targetFactory.getProxy());
367392
proxyFactory.addInterface(TestWithInterface.class);
368393
proxyFactory.addAdvice(this.ti);
369-
370394
TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy();
371395

372396
proxy.doSomething();
@@ -395,7 +419,6 @@ public void withInterfaceOnTargetCglibProxy() {
395419
proxyFactory.setTarget(targetFactory.getProxy());
396420
proxyFactory.addInterface(TestWithInterface.class);
397421
proxyFactory.addAdvice(this.ti);
398-
399422
TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy();
400423

401424
proxy.doSomething();
@@ -544,6 +567,7 @@ public void doSomethingElseWithCheckedExceptionAndRollbackRule() throws Exceptio
544567
}
545568
}
546569

570+
547571
@Transactional
548572
public static class TestWithReactive {
549573

@@ -564,6 +588,37 @@ public Flux<Object> fluxFailure() {
564588
}
565589
}
566590

591+
592+
@Transactional
593+
public static class TestWithCompletableFuture {
594+
595+
public CompletableFuture<String> doSomething() {
596+
assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue();
597+
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isFalse();
598+
return CompletableFuture.completedFuture("ok");
599+
}
600+
601+
public CompletableFuture<String> doSomethingErroneous() {
602+
assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue();
603+
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isFalse();
604+
return CompletableFuture.failedFuture(new IllegalStateException());
605+
}
606+
607+
public CompletableFuture<String> doSomethingErroneousWithCheckedException() {
608+
assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue();
609+
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isFalse();
610+
return CompletableFuture.failedFuture(new Exception());
611+
}
612+
613+
@Transactional(rollbackFor = Exception.class)
614+
public CompletableFuture<String> doSomethingErroneousWithCheckedExceptionAndRollbackRule() {
615+
assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue();
616+
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isFalse();
617+
return CompletableFuture.failedFuture(new Exception());
618+
}
619+
}
620+
621+
567622
@Transactional
568623
public static class TestWithVavrTry {
569624

0 commit comments

Comments
 (0)