Pass FailsafeExecutor.with(Executor) calls for ExecutorService instances

Part of issue #311

Author: jhalterman

CHANGELOG.md

@@ -1,3 +1,9 @@
+# 3.0.2
+
+### Bug Fixes
+
+- Issue #311 - `with(Executor)` not working as expected in some cases.
+
 # 3.0.1
 
 ### Improvements

src/main/java/dev/failsafe/FailsafeExecutor.java

@@ -385,17 +385,26 @@ public FailsafeExecutor<R> with(ExecutorService executorService) {
   }
 
   /**
-   * Configures the {@code executor} to use as a wrapper around executions. The {@code executor} is responsible for
-   * propagating executions. Executions that normally return a result, such as {@link #get(CheckedSupplier)} will return
-   * {@code null} since the {@link Executor} interface does not support results.
+   * Configures the {@code executor} to use as a wrapper around executions. If the {@code executor} is actually an
+   * instance of {@link ExecutorService}, then the {@code executor} will be configured via {@link
+   * #with(ExecutorService)} instead.
+   * <p>
+   * The {@code executor} is responsible for propagating executions. Executions that normally return a result, such as
+   * {@link #get(CheckedSupplier)} will return {@code null} since the {@link Executor} interface does not support
+   * results.
+   * </p>
    * <p>The {@code executor} will not be used for {@link #getStageAsync(CheckedSupplier) getStageAsync} calls since
    * those require a returned result.
    * </p>
    *
    * @throws NullPointerException if {@code executor} is null
    */
   public FailsafeExecutor<R> with(Executor executor) {
-    this.executor = Assert.notNull(executor, "executor");
+    Assert.notNull(executor, "executor");
+    if (executor instanceof ExecutorService)
+      with((ExecutorService) executor);
+    else
+      this.executor = executor;
     return this;
   }
 

src/main/java/dev/failsafe/spi/Scheduler.java

@@ -48,6 +48,8 @@ static Scheduler of(ScheduledExecutorService scheduledExecutorService) {
    * Returns a Scheduler adapted from the {@code executorService}.
    */
   static Scheduler of(ExecutorService executorService) {
-    return new DelegatingScheduler(executorService);
+    return executorService instanceof ScheduledExecutorService ?
+      of((ScheduledExecutorService) executorService) :
+      new DelegatingScheduler(executorService);
   }
 }

src/test/java/dev/failsafe/issues/Issue311Test.java

@@ -0,0 +1,26 @@
+package dev.failsafe.issues;
+
+import dev.failsafe.Failsafe;
+import dev.failsafe.RetryPolicy;
+import org.testng.annotations.Test;
+
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.testng.Assert.assertEquals;
+
+public class Issue311Test {
+  @Test
+  void failsafeFail() throws Throwable {
+    AtomicInteger counter = new AtomicInteger(0);
+    Executor executor = Executors.newSingleThreadExecutor();
+    Failsafe.with(RetryPolicy.builder().handle(RuntimeException.class).withMaxAttempts(2).build())
+      .with(executor)
+      .runAsync(() -> {
+        if (counter.incrementAndGet() == 1)
+          throw new RuntimeException();
+      }).get();
+    assertEquals(counter.get(), 2);
+  }
+}