[Bug #314] Add tests ensuring proper ThrottledFuture#then callbacks e…

…xecution when future is cancelled.

src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java

@@ -10,6 +10,8 @@ public interface ChainableFuture<T, F extends ChainableFuture<T, F>> extends Fut
      * Action is executed no matter if the future is completed successfully, exceptionally or cancelled.
      * <p>
      * If the future is already completed, it is executed synchronously.
+     * <p>
+     * Note that you <b>must</b> use the future passed as the parameter and not the original future object.
      * @param action action receiving this future after completion
      * @return this future
      */

src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java

@@ -89,11 +89,12 @@ public ThrottledFuture<T> setCachedResult(@Nullable final T cachedResult) {
 
     @Override
     public boolean cancel(boolean mayInterruptIfRunning) {
+        final boolean wasCanceled = isCancelled();
         if(!future.cancel(mayInterruptIfRunning)) {
             return false;
         }
 
-        if (task != null) {
+        if (!wasCanceled && task != null) {
             callbackLock.lock();
             onCompletion.forEach(c -> c.accept(this));
             callbackLock.unlock();
@@ -268,7 +269,7 @@ public ThrottledFuture<T> then(Consumer<ThrottledFuture<T>> action) {
 
     /**
      * @return {@code true} if this future completed
-     * exceptionally
+     * exceptionally or was cancelled.
      */
     public boolean isCompletedExceptionally() {
         return future.isCompletedExceptionally();

src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java

@@ -137,7 +137,59 @@ void thenActionIsExecutedOnceFutureIsCancelled() {
     }
 
     @Test
-    void thenActionIsExecutedWhenFutureCompletedExceptionally() {
+    void thenActionIsExecutedOnlyOnceWhenFutureIsCancelled() {
+        final AtomicInteger executionCount = new AtomicInteger(0);
+        final ThrottledFuture<?> future = ThrottledFuture.of(() -> null);
+        future.then(f -> executionCount.incrementAndGet());
+        assertEquals(0, executionCount.get());
+        future.cancel(false);
+        assertEquals(1, executionCount.get());
+        future.cancel(false);
+        future.cancel(true);
+        assertEquals(1, executionCount.get());
+    }
+
+    @Test
+    void thenActionIsExecutedWhenFutureCompletesExceptionally() {
+        final AtomicBoolean completed = new AtomicBoolean(false);
+        final ThrottledFuture<?> future = ThrottledFuture.of(() -> {
+            throw new RuntimeException();
+        });
+        future.then(futureResult -> completed.set(true));
+        assertFalse(completed.get());
+        future.run(null);
+        assertTrue(completed.get());
+    }
+
+    @Test
+    void isCompletedExceptionallyReturnsTrueWhenFutureCompletesExceptionally() {
+        final ThrottledFuture<?> future = ThrottledFuture.of(() -> {
+            throw new RuntimeException();
+        });
+        future.run(null);
+        assertTrue(future.isCompletedExceptionally());
+    }
+
+    @Test
+    void isCompletedExceptionallyReturnsFalseWhenFutureCompletesNormally() {
+        final ThrottledFuture<?> future = ThrottledFuture.of(() -> null);
+        future.run(null);
+        assertFalse(future.isCompletedExceptionally());
+        assertFalse(future.isCancelled());
+        assertTrue(future.isDone());
+    }
+
+    @Test
+    void isCompletedExceptionallyReturnsTrueWhenFutureIsCancelled() {
+        final ThrottledFuture<?> future = ThrottledFuture.of(() -> null);
+        future.cancel(false);
+        assertTrue(future.isCompletedExceptionally());
+        assertTrue(future.isCancelled());
+        assertTrue(future.isDone());
+    }
+
+    @Test
+    void thenActionIsExecutedWhenFutureIsAlreadyCompletedExceptionally() {
         final AtomicBoolean completed = new AtomicBoolean(false);
         final ThrottledFuture<?> future = ThrottledFuture.of(() -> {
             throw new RuntimeException();