From 7bc38cb0a8a5261fd34414fc5d6fc01d19fa6d3d Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 28 Jul 2015 21:09:39 +1000 Subject: [PATCH 1/8] Support blocking with: `scalaFuture.toJava.toCompletableFuture` I believe this implements the somewhat reluctant consensus reached in #43. Go with the flow by allowing people to block using the standard Java 8 idiom of `toCompletableFuture.get`. I've added a comment and a test to caution that calling `toCompletableFuture.complete(value)` does *not* effect the underlying Promise. Fixes #43 --- .../java8/FutureConvertersImpl.scala | 11 +++++++-- .../compat/java8/FutureConvertersTest.java | 23 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala index 5c907e7..48c861c 100644 --- a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala +++ b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala @@ -66,8 +66,15 @@ object FuturesConvertersImpl { cf } - override def toCompletableFuture(): CompletableFuture[T] = - throw new UnsupportedOperationException("this CompletionStage represents a read-only Scala Future") + /** + * @inheritdoc + * + * WARNING: completing the result of this method will not complete the underlying + * Scala Future or Promise. + */ + override def toCompletableFuture(): CompletableFuture[T] = { + this // TODO or maybe `thenApply(JF.identity())` + } override def toString: String = super[CompletableFuture].toString } diff --git a/src/test/java/scala/compat/java8/FutureConvertersTest.java b/src/test/java/scala/compat/java8/FutureConvertersTest.java index a8d4861..aaaccc0 100644 --- a/src/test/java/scala/compat/java8/FutureConvertersTest.java +++ b/src/test/java/scala/compat/java8/FutureConvertersTest.java @@ -318,4 +318,27 @@ public void testToJavaExceptionally() throws InterruptedException, latch.countDown(); assertEquals("Hello", second.toCompletableFuture().get()); } + + @Test + public void testToJavaToCompletableFuture() throws ExecutionException, InterruptedException { + final Promise p = promise(); + final CompletionStage cs = toJava(p.future()); + CompletableFuture cf = cs.toCompletableFuture(); + assertEquals("notyet", cf.getNow("notyet")); + p.success("done"); + assertEquals("done", cf.get()); + } + + @Test + public void testToJavaToCompletableFutureDoesNotMutateUnderlyingPromise() throws ExecutionException, InterruptedException { + final Promise p = promise(); + Future sf = p.future(); + final CompletionStage cs = toJava(sf); + CompletableFuture cf = cs.toCompletableFuture(); + assertEquals("notyet", cf.getNow("notyet")); + cf.complete("done"); + assertEquals("done", cf.get()); + assertFalse(sf.isCompleted()); + assertFalse(p.isCompleted()); + } } From 46d8b308701271c606830cd859dbe72370810568 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 28 Jul 2015 22:10:42 +1000 Subject: [PATCH 2/8] Test case for toJava / thenCompose that now works Since `toCompletableFuture` stopped throwing an `UnsupportedOperationException` in the previous commit, this use case is now supported. Fixes #29 --- .../compat/java8/FutureConvertersTest.java | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/test/java/scala/compat/java8/FutureConvertersTest.java b/src/test/java/scala/compat/java8/FutureConvertersTest.java index aaaccc0..81b0d5f 100644 --- a/src/test/java/scala/compat/java8/FutureConvertersTest.java +++ b/src/test/java/scala/compat/java8/FutureConvertersTest.java @@ -7,11 +7,9 @@ import scala.concurrent.Future; import scala.concurrent.Promise; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; +import java.util.concurrent.*; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.Assert.*; import static scala.compat.java8.FutureConverters.*; @@ -319,6 +317,30 @@ public void testToJavaExceptionally() throws InterruptedException, assertEquals("Hello", second.toCompletableFuture().get()); } + @Test + public void testToJavaThenComposeWithToJavaThenAccept() throws InterruptedException, + ExecutionException, TimeoutException { + final Promise p1 = promise(); + final CompletableFuture future = new CompletableFuture<>(); + + final CompletionStage second = + CompletableFuture.supplyAsync(() -> "Hello"). + thenCompose(x -> toJava(p1.future())); + + second.handle((x, t) -> { + if (t != null) { + t.printStackTrace(); + future.completeExceptionally(t); + } else { + future.complete(x); + } + return null; + }); + + p1.success("Hello"); + assertEquals("Hello", future.get(1000, MILLISECONDS)); + } + @Test public void testToJavaToCompletableFuture() throws ExecutionException, InterruptedException { final Promise p = promise(); From 9cb51e64e061de57a88f34a69ac409fe21a8af20 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 29 Jul 2015 10:33:49 +1000 Subject: [PATCH 3/8] Extra test and comment suggested during review --- .../concurrent/java8/FutureConvertersImpl.scala | 2 +- .../scala/compat/java8/FutureConvertersTest.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala index 48c861c..c7d29b0 100644 --- a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala +++ b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala @@ -70,7 +70,7 @@ object FuturesConvertersImpl { * @inheritdoc * * WARNING: completing the result of this method will not complete the underlying - * Scala Future or Promise. + * Scala Future or Promise (ie, the one that that was passed to `toJava`.) */ override def toCompletableFuture(): CompletableFuture[T] = { this // TODO or maybe `thenApply(JF.identity())` diff --git a/src/test/java/scala/compat/java8/FutureConvertersTest.java b/src/test/java/scala/compat/java8/FutureConvertersTest.java index 81b0d5f..a06fc3c 100644 --- a/src/test/java/scala/compat/java8/FutureConvertersTest.java +++ b/src/test/java/scala/compat/java8/FutureConvertersTest.java @@ -363,4 +363,17 @@ public void testToJavaToCompletableFutureDoesNotMutateUnderlyingPromise() throws assertFalse(sf.isCompleted()); assertFalse(p.isCompleted()); } + + @Test + public void testToJavaToCompletableFutureJavaCompleteCallledAfterScalaComplete() throws ExecutionException, InterruptedException { + final Promise p = promise(); + Future sf = p.future(); + final CompletionStage cs = toJava(sf); + CompletableFuture cf = cs.toCompletableFuture(); + assertEquals("notyet", cf.getNow("notyet")); + p.success("scaladone"); + assertEquals("scaladone", cf.get()); + cf.complete("javadone"); + assertEquals("scaladone", cf.get()); + } } From 976605e7134cbd079af297a171277f14eec9f3b0 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 13 Aug 2015 15:39:05 +1000 Subject: [PATCH 4/8] Simplify and not the source of a test case --- .../compat/java8/FutureConvertersTest.java | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/test/java/scala/compat/java8/FutureConvertersTest.java b/src/test/java/scala/compat/java8/FutureConvertersTest.java index a06fc3c..4f0421f 100644 --- a/src/test/java/scala/compat/java8/FutureConvertersTest.java +++ b/src/test/java/scala/compat/java8/FutureConvertersTest.java @@ -320,23 +320,12 @@ public void testToJavaExceptionally() throws InterruptedException, @Test public void testToJavaThenComposeWithToJavaThenAccept() throws InterruptedException, ExecutionException, TimeoutException { + // Test case from https://github.com/scala/scala-java8-compat/issues/29 final Promise p1 = promise(); final CompletableFuture future = new CompletableFuture<>(); - final CompletionStage second = - CompletableFuture.supplyAsync(() -> "Hello"). - thenCompose(x -> toJava(p1.future())); - - second.handle((x, t) -> { - if (t != null) { - t.printStackTrace(); - future.completeExceptionally(t); - } else { - future.complete(x); - } - return null; - }); - + CompletableFuture.supplyAsync(() -> "Hello"). + thenCompose(x -> toJava(p1.future())).handle((x, t) -> future.complete(x)); p1.success("Hello"); assertEquals("Hello", future.get(1000, MILLISECONDS)); } From 4b85f72c433ef9b6a1c94d60ad474c9e5223205b Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 13 Aug 2015 15:42:18 +1000 Subject: [PATCH 5/8] Test case to show completion from Java future before Scala future --- .../scala/compat/java8/FutureConvertersTest.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/test/java/scala/compat/java8/FutureConvertersTest.java b/src/test/java/scala/compat/java8/FutureConvertersTest.java index 4f0421f..12fc108 100644 --- a/src/test/java/scala/compat/java8/FutureConvertersTest.java +++ b/src/test/java/scala/compat/java8/FutureConvertersTest.java @@ -354,7 +354,7 @@ public void testToJavaToCompletableFutureDoesNotMutateUnderlyingPromise() throws } @Test - public void testToJavaToCompletableFutureJavaCompleteCallledAfterScalaComplete() throws ExecutionException, InterruptedException { + public void testToJavaToCompletableFutureJavaCompleteCalledAfterScalaComplete() throws ExecutionException, InterruptedException { final Promise p = promise(); Future sf = p.future(); final CompletionStage cs = toJava(sf); @@ -365,4 +365,17 @@ public void testToJavaToCompletableFutureJavaCompleteCallledAfterScalaComplete() cf.complete("javadone"); assertEquals("scaladone", cf.get()); } + + @Test + public void testToJavaToCompletableFutureJavaCompleteCalledBeforeScalaComplete() throws ExecutionException, InterruptedException { + final Promise p = promise(); + Future sf = p.future(); + final CompletionStage cs = toJava(sf); + CompletableFuture cf = cs.toCompletableFuture(); + assertEquals("notyet", cf.getNow("notyet")); + cf.complete("javadone"); + assertEquals("javadone", cf.get()); + p.success("scaladone"); + assertEquals("javadone", cf.get()); + } } From fcdda1a91479d78a49c9b4921b531a788b407b38 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 13 Aug 2015 20:32:12 +1000 Subject: [PATCH 6/8] Remove vague TODO comment. I wasn't sure which way to go here, but now we have a few more tests I guess this comment doesn't really help anyone. --- .../scala/scala/concurrent/java8/FutureConvertersImpl.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala index c7d29b0..fff1986 100644 --- a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala +++ b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala @@ -73,7 +73,7 @@ object FuturesConvertersImpl { * Scala Future or Promise (ie, the one that that was passed to `toJava`.) */ override def toCompletableFuture(): CompletableFuture[T] = { - this // TODO or maybe `thenApply(JF.identity())` + this } override def toString: String = super[CompletableFuture].toString From 385de85dbfbff40c630ef6dcf6c39438db21839d Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 13 Aug 2015 20:42:22 +1000 Subject: [PATCH 7/8] Use `s.c.blocking` in `future.toJava.get` As suggested by Viktor. --- .../scala/scala/concurrent/java8/FutureConvertersImpl.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala index fff1986..7e77fa4 100644 --- a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala +++ b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala @@ -6,7 +6,7 @@ package scala.concurrent.java8 // Located in this package to access private[concurrent] members import scala.concurrent.{ Future, Promise, ExecutionContext, ExecutionContextExecutorService, ExecutionContextExecutor, impl } -import java.util.concurrent.{ CompletionStage, Executor, ExecutorService, CompletableFuture } +import java.util.concurrent._ import scala.util.{ Try, Success, Failure } import java.util.function.{ BiConsumer, Function ⇒ JF, Consumer, BiFunction } @@ -76,6 +76,10 @@ object FuturesConvertersImpl { this } + override def get(): T = scala.concurrent.blocking(super.get()) + + override def get(timeout: Long, unit: TimeUnit): T = scala.concurrent.blocking(super.get(timeout, unit)) + override def toString: String = super[CompletableFuture].toString } } From 1d60cd0aca03c9721c0d597c7251db963a967d74 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 17 Aug 2015 14:54:15 +1000 Subject: [PATCH 8/8] Make toJava(scalaFuture).toCF.obtrude* unsupported The status quo was to allow the wrapper and the wrapped to diverge; the as shown by: ``` @Test public void testToJavaToCompletableFutureJavaObtrudeCalledAfterScalaComplete() throws Exception { final Promise p = promise(); Future sf = p.future(); final CompletionStage cs = toJava(sf); CompletableFuture cf = cs.toCompletableFuture(); assertEquals("notyet", cf.getNow("notyet")); p.success("scaladone"); assertEquals("scaladone", cf.get()); cf.obtrudeValue("javadone"); assertEquals("javadone", cf.get()); // obtrude does not mutate the underlying Scala future assertEquals("scaladone", Await.result(sf, Duration.Inf())); } ``` This commit throws an UOE, preferring to fail fast. --- .../java8/FutureConvertersImpl.scala | 8 +++++--- .../compat/java8/FutureConvertersTest.java | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala index 7e77fa4..1ee934e 100644 --- a/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala +++ b/src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala @@ -72,9 +72,11 @@ object FuturesConvertersImpl { * WARNING: completing the result of this method will not complete the underlying * Scala Future or Promise (ie, the one that that was passed to `toJava`.) */ - override def toCompletableFuture(): CompletableFuture[T] = { - this - } + override def toCompletableFuture(): CompletableFuture[T] = this + + override def obtrudeValue(value: T): Unit = throw new UnsupportedOperationException("obtrudeValue may not be used on the result of toJava(scalaFuture)") + + override def obtrudeException(ex: Throwable): Unit = throw new UnsupportedOperationException("obtrudeException may not be used on the result of toJava(scalaFuture)") override def get(): T = scala.concurrent.blocking(super.get()) diff --git a/src/test/java/scala/compat/java8/FutureConvertersTest.java b/src/test/java/scala/compat/java8/FutureConvertersTest.java index 12fc108..7735aba 100644 --- a/src/test/java/scala/compat/java8/FutureConvertersTest.java +++ b/src/test/java/scala/compat/java8/FutureConvertersTest.java @@ -378,4 +378,24 @@ public void testToJavaToCompletableFutureJavaCompleteCalledBeforeScalaComplete() p.success("scaladone"); assertEquals("javadone", cf.get()); } + + @Test + public void testToJavaToCompletableFutureJavaObtrudeCalledBeforeScalaComplete() throws ExecutionException, InterruptedException { + final Promise p = promise(); + Future sf = p.future(); + final CompletionStage cs = toJava(sf); + CompletableFuture cf = cs.toCompletableFuture(); + try { + cf.obtrudeValue(""); + fail(); + } catch (UnsupportedOperationException iae) { + // okay + } + try { + cf.obtrudeException(new Exception()); + fail(); + } catch (UnsupportedOperationException iae) { + // okay + } + } }