From 9e57a27ec4e66db9b81dc0af6ff290521b791904 Mon Sep 17 00:00:00 2001
From: Ladislav Thon <lthon@redhat.com>
Date: Wed, 30 Oct 2024 11:33:16 +0100
Subject: [PATCH] Fix asynchronous @BeforeRetry

The async implementation of retry had a bug in that it attempted to call
the before retry action even before the initial attempt. This is wrong,
plus it leads to an exception in the constructor of `FailureContext`.
This commit fixes that bug and adds missing tests.
---
 .../core/retry/CompletionStageRetry.java      |  2 +-
 .../retry/beforeretry/AsyncHelloService.java  | 39 +++++++++++++++++++
 .../AsynchronousCompletionStageRetryTest.java | 19 +++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 testsuite/basic/src/test/java/io/smallrye/faulttolerance/async/compstage/retry/beforeretry/AsyncHelloService.java
 create mode 100644 testsuite/basic/src/test/java/io/smallrye/faulttolerance/async/compstage/retry/beforeretry/AsynchronousCompletionStageRetryTest.java

diff --git a/implementation/core/src/main/java/io/smallrye/faulttolerance/core/retry/CompletionStageRetry.java b/implementation/core/src/main/java/io/smallrye/faulttolerance/core/retry/CompletionStageRetry.java
index 170916493..e026f717c 100644
--- a/implementation/core/src/main/java/io/smallrye/faulttolerance/core/retry/CompletionStageRetry.java
+++ b/implementation/core/src/main/java/io/smallrye/faulttolerance/core/retry/CompletionStageRetry.java
@@ -87,7 +87,7 @@ private CompletionStage<V> afterDelay(InvocationContext<CompletionStage<V>> ctx,
             }
         }
 
-        if (beforeRetry != null) {
+        if (beforeRetry != null && attempt > 0) {
             try {
                 beforeRetry.accept(new FailureContext(lastFailure, ctx));
             } catch (Exception e) {
diff --git a/testsuite/basic/src/test/java/io/smallrye/faulttolerance/async/compstage/retry/beforeretry/AsyncHelloService.java b/testsuite/basic/src/test/java/io/smallrye/faulttolerance/async/compstage/retry/beforeretry/AsyncHelloService.java
new file mode 100644
index 000000000..57e10b608
--- /dev/null
+++ b/testsuite/basic/src/test/java/io/smallrye/faulttolerance/async/compstage/retry/beforeretry/AsyncHelloService.java
@@ -0,0 +1,39 @@
+package io.smallrye.faulttolerance.async.compstage.retry.beforeretry;
+
+import static java.util.concurrent.CompletableFuture.completedFuture;
+import static java.util.concurrent.CompletableFuture.failedFuture;
+
+import java.io.IOException;
+import java.util.concurrent.CompletionStage;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import jakarta.enterprise.context.ApplicationScoped;
+
+import org.eclipse.microprofile.faulttolerance.Asynchronous;
+import org.eclipse.microprofile.faulttolerance.Fallback;
+import org.eclipse.microprofile.faulttolerance.Retry;
+
+import io.smallrye.faulttolerance.api.BeforeRetry;
+
+@ApplicationScoped
+public class AsyncHelloService {
+    static final AtomicInteger COUNTER = new AtomicInteger(0);
+    static final AtomicInteger BEFORE_RETRY_COUNTER = new AtomicInteger(0);
+
+    @Asynchronous
+    @Retry(maxRetries = 2)
+    @BeforeRetry(methodName = "beforeRetry")
+    @Fallback(fallbackMethod = "fallback")
+    public CompletionStage<String> hello() {
+        COUNTER.incrementAndGet();
+        return failedFuture(new IOException("Simulated IO error"));
+    }
+
+    private void beforeRetry() {
+        BEFORE_RETRY_COUNTER.incrementAndGet();
+    }
+
+    private CompletionStage<String> fallback() {
+        return completedFuture("Fallback");
+    }
+}
diff --git a/testsuite/basic/src/test/java/io/smallrye/faulttolerance/async/compstage/retry/beforeretry/AsynchronousCompletionStageRetryTest.java b/testsuite/basic/src/test/java/io/smallrye/faulttolerance/async/compstage/retry/beforeretry/AsynchronousCompletionStageRetryTest.java
new file mode 100644
index 000000000..137cb3e22
--- /dev/null
+++ b/testsuite/basic/src/test/java/io/smallrye/faulttolerance/async/compstage/retry/beforeretry/AsynchronousCompletionStageRetryTest.java
@@ -0,0 +1,19 @@
+package io.smallrye.faulttolerance.async.compstage.retry.beforeretry;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.concurrent.ExecutionException;
+
+import org.junit.jupiter.api.Test;
+
+import io.smallrye.faulttolerance.util.FaultToleranceBasicTest;
+
+@FaultToleranceBasicTest
+public class AsynchronousCompletionStageRetryTest {
+    @Test
+    public void testAsyncBeforeRetry(AsyncHelloService helloService) throws InterruptedException, ExecutionException {
+        assertThat(helloService.hello().toCompletableFuture().get()).isEqualTo("Fallback");
+        assertThat(AsyncHelloService.COUNTER.get()).isEqualTo(3);
+        assertThat(AsyncHelloService.BEFORE_RETRY_COUNTER.get()).isEqualTo(2);
+    }
+}