Add few improvements/fixes (matching guava): (googleapis#29)

vam-google · web-flow · commit 216b72a8927a · 2017-06-14T08:52:01.000-07:00
1) Add ApiFutures.addCallback() method overloaded version which accepts executor
2) Add ApiFutures.immediateCancelledFuture() method
3) Remove "settability" from AbstractApiFuture and make that functionality available only in SettableApiFuture settable.
diff --git a/src/main/java/com/google/api/core/AbstractApiFuture.java b/src/main/java/com/google/api/core/AbstractApiFuture.java
@@ -70,11 +70,11 @@ public boolean isDone() {
     return impl.isDone();
   }
 
-  public boolean set(V value) {
+  protected boolean set(V value) {
     return impl.set(value);
   }
 
-  public boolean setException(Throwable throwable) {
+  protected boolean setException(Throwable throwable) {
     return impl.setException(throwable);
   }
 
@@ -87,12 +87,12 @@ ListenableFuture<V> getInternalListenableFuture() {
 
   private class InternalSettableFuture extends AbstractFuture<V> {
     @Override
-    public boolean set(@Nullable V value) {
+    protected boolean set(@Nullable V value) {
       return super.set(value);
     }
 
     @Override
-    public boolean setException(Throwable throwable) {
+    protected boolean setException(Throwable throwable) {
       return super.setException(throwable);
     }
 
diff --git a/src/main/java/com/google/api/core/ApiFutures.java b/src/main/java/com/google/api/core/ApiFutures.java
@@ -30,13 +30,16 @@
  */
 package com.google.api.core;
 
+import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
+
 import com.google.common.base.Function;
 import com.google.common.collect.Iterables;
 import com.google.common.util.concurrent.AsyncFunction;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.List;
+import java.util.concurrent.Executor;
 import javax.annotation.Nullable;
 
 /** Static utility methods for the {@link ApiFuture} interface. */
@@ -45,6 +48,11 @@ private ApiFutures() {}
 
   public static <V> void addCallback(
       final ApiFuture<V> future, final ApiFutureCallback<? super V> callback) {
+    addCallback(future, callback, directExecutor());
+  }
+
+  public static <V> void addCallback(
+      final ApiFuture<V> future, final ApiFutureCallback<? super V> callback, Executor executor) {
     Futures.addCallback(
         listenableFutureForApiFuture(future),
         new FutureCallback<V>() {
@@ -57,7 +65,8 @@ public void onFailure(Throwable t) {
           public void onSuccess(V v) {
             callback.onSuccess(v);
           }
-        });
+        },
+        executor);
   }
 
   public static <V, X extends Throwable> ApiFuture<V> catching(
@@ -80,6 +89,10 @@ public static <V> ApiFuture<V> immediateFailedFuture(Throwable throwable) {
     return new ListenableFutureToApiFuture<V>(Futures.<V>immediateFailedFuture(throwable));
   }
 
+  public static <V> ApiFuture<V> immediateCancelledFuture() {
+    return new ListenableFutureToApiFuture<V>(Futures.<V>immediateCancelledFuture());
+  }
+
   public static <V, X> ApiFuture<X> transform(
       ApiFuture<? extends V> input, final ApiFunction<? super V, ? extends X> function) {
     return new ListenableFutureToApiFuture<>(
diff --git a/src/main/java/com/google/api/core/SettableApiFuture.java b/src/main/java/com/google/api/core/SettableApiFuture.java
@@ -42,4 +42,14 @@ private SettableApiFuture() {}
   public static <V> SettableApiFuture<V> create() {
     return new SettableApiFuture<>();
   }
+
+  @Override
+  protected boolean set(V value) {
+    return super.set(value);
+  }
+
+  @Override
+  protected boolean setException(Throwable throwable) {
+    return super.setException(throwable);
+  }
 }
diff --git a/src/test/java/com/google/api/core/ApiFuturesTest.java b/src/test/java/com/google/api/core/ApiFuturesTest.java
@@ -30,9 +30,12 @@
  */
 package com.google.api.core;
 
+import static com.google.common.truth.Truth.assertThat;
+
 import com.google.common.collect.ImmutableList;
-import com.google.common.truth.Truth;
 import java.util.List;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.Test;
 
@@ -56,7 +59,7 @@ public void onFailure(Throwable t) {
           }
         });
     future.set(0);
-    Truth.assertThat(flag.get()).isEqualTo(1);
+    assertThat(flag.get()).isEqualTo(1);
   }
 
   @Test
@@ -73,7 +76,7 @@ public Integer apply(Exception ex) {
               }
             });
     future.setException(new Exception());
-    Truth.assertThat(fallback.get()).isEqualTo(42);
+    assertThat(fallback.get()).isEqualTo(42);
   }
 
   @Test
@@ -89,7 +92,7 @@ public String apply(Integer input) {
               }
             });
     inputFuture.set(6);
-    Truth.assertThat(transformedFuture.get()).isEqualTo("6");
+    assertThat(transformedFuture.get()).isEqualTo("6");
   }
 
   @Test
@@ -100,7 +103,7 @@ public void testAllAsList() throws Exception {
         ApiFutures.allAsList(ImmutableList.of(inputFuture1, inputFuture2));
     inputFuture1.set(1);
     inputFuture2.set(2);
-    Truth.assertThat(listFuture.get()).containsExactly(1, 2).inOrder();
+    assertThat(listFuture.get()).containsExactly(1, 2).inOrder();
   }
 
   @Test
@@ -115,6 +118,32 @@ public ApiFuture<Integer> apply(Integer input) {
                 return ApiFutures.immediateFuture(input + 1);
               }
             });
-    Truth.assertThat(outputFuture.get()).isEqualTo(1);
+    assertThat(outputFuture.get()).isEqualTo(1);
+  }
+
+  @Test
+  public void testImmediateFailedFuture() throws InterruptedException {
+    ApiFuture<String> future =
+        ApiFutures.immediateFailedFuture(new IllegalArgumentException("The message"));
+    IllegalArgumentException exception = null;
+    try {
+      future.get();
+    } catch (ExecutionException e) {
+      exception = (IllegalArgumentException) e.getCause();
+    }
+    assertThat(exception).isNotNull();
+    assertThat(exception.getMessage()).isEqualTo("The message");
+  }
+
+  @Test
+  public void testImmediateCancelledFuture() throws InterruptedException, ExecutionException {
+    ApiFuture<String> future = ApiFutures.immediateCancelledFuture();
+    CancellationException exception = null;
+    try {
+      future.get();
+    } catch (CancellationException e) {
+      exception = e;
+    }
+    assertThat(exception).isNotNull();
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -70,11 +70,11 @@ public boolean isDone() {`
`70`	`70`	`return impl.isDone();`
`71`	`71`	`}`
`72`	`72`
`73`		`- public boolean set(V value) {`
	`73`	`+ protected boolean set(V value) {`
`74`	`74`	`return impl.set(value);`
`75`	`75`	`}`
`76`	`76`
`77`		`- public boolean setException(Throwable throwable) {`
	`77`	`+ protected boolean setException(Throwable throwable) {`
`78`	`78`	`return impl.setException(throwable);`
`79`	`79`	`}`
`80`	`80`
`@@ -87,12 +87,12 @@ ListenableFuture<V> getInternalListenableFuture() {`
`87`	`87`
`88`	`88`	`private class InternalSettableFuture extends AbstractFuture<V> {`
`89`	`89`	`@Override`
`90`		`- public boolean set(@Nullable V value) {`
	`90`	`+ protected boolean set(@Nullable V value) {`
`91`	`91`	`return super.set(value);`
`92`	`92`	`}`
`93`	`93`
`94`	`94`	`@Override`
`95`		`- public boolean setException(Throwable throwable) {`
	`95`	`+ protected boolean setException(Throwable throwable) {`
`96`	`96`	`return super.setException(throwable);`
`97`	`97`	`}`
`98`	`98`