Support blocking with: scalaFuture.toJava.toCompletableFuture
#49
Conversation
| /** | ||
| * @inheritdoc | ||
| * | ||
| * WARNING: completing the result of this method will not complete the underlying |
There was a problem hiding this comment.
There is no underlying Scala Future or Promise for this class? (CF)
There was a problem hiding this comment.
What happens if the CF is completed by a Scala Future prior to toCompletableFuture.complete(x) gets called?
There was a problem hiding this comment.
There is no underlying Scala Future or Promise for this class? (CF)
The underlying future is the one passed to toJava:
def toJava[T](f: Future[T]): CompletionStage[T] = {
val cf = new CF[T]
implicit val ec = InternalCallbackExecutor
f onComplete cf
cf
}
In my other PR, that also becomes a field of CF so that we can efficiently unwrap it.
There was a problem hiding this comment.
What happens if the
CFis completed by a Scala Future prior totoCompletableFuture.complete(x)gets called?
I've added a test case to show what happens, could you please review it?
There was a problem hiding this comment.
That test looks great.
What happens if obtrudeX is called?
|
One thought, shouldn't |
Sounds legit. Does my last commit put this in the right spot? |
|
@viktorklang Please see my latest commit that addresses the |
I believe this implements the somewhat reluctant consensus reached in scala#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 scala#43
Since `toCompletableFuture` stopped throwing an `UnsupportedOperationException` in the previous commit, this use case is now supported. Fixes scala#29
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.
As suggested by Viktor.
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<String> p = promise();
Future<String> sf = p.future();
final CompletionStage<String> cs = toJava(sf);
CompletableFuture<String> 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.
Support blocking with: `scalaFuture.toJava.toCompletableFuture`
Thought I'd get the ball rolling with a PR.
Review by @jroper / @viktorklang / @rkuhn (as available)