Hi Pavel,
Looks fine.
WebSocket.java:
- line 82: I'd replace 'never' with 'does not'; it is more like a
statement for fact and less a guarantee.
- LIne 54: The notation "{@code .onX}" seems unnecessary, is quite
cryptic, and can be removed.
Are there any tests that would be updated to go along with the change to
CF<WebSocket>.
Regards, Roger
On 6/7/2016 11:48 AM, Pavel Rappo wrote:
Hi,
Could you please review the following change for JDK-8156693?
http://cr.openjdk.java.net/~prappo/8156693/webrev.01/
This change addresses some WebSocket API refinements and enhancements from [1].
WebSocket returns `CompletableFuture` from some of its methods. CFs are used by
an implementation to signal when the operation has completed.
Currently, there is a number of known issues with how it's done. These issues
reduce usability of the feature.
1. Change methods that return `CompletableFuture<Void>` to return
`CompletableFuture<WebSocket>`.
The design was based on a false assumption that `WebSocket` (possibly needed to
continue asynchronous operation) is always accessible in the same scope where
the returned CF is used.
Before:
CompletableFuture<WebSocket> wsf = webSocketBuilder.buildAsync();
wsf.thenCompose(ws -> ws.sendText("Hi ", false))
.thenCompose((v) -> wsf.join().sendText("there", false)).join();
After:
CompletableFuture<WebSocket> wsf = webSocketBuilder.buildAsync();
wsf.thenCompose(ws -> ws.sendText("Hi ", false))
.thenCompose(ws -> ws.sendText("there", false)).join();
2. Remove stipulation that `CompletableFuture` is returned immediately.
This is needlessly restrictive for an implementation. It might also give a false
impression that the method *always* dispatches to another thread and returns
immediately.
3. Make methods that return `CompletableFuture` to relay (as mush as possible)
recoverable or non-programming exceptions through returned CF.
There's some evidence [2], [3] that this aspect actually makes it harder to use
such methods in completely async computations.
Thanks,
-Pavel
--------------------------------------------------------------------------------
[1] https://bugs.openjdk.java.net/browse/JDK-8155621
[2] http://mail.openjdk.java.net/pipermail/net-dev/2016-April/009638.html
[3] http://cs.oswego.edu/pipermail/concurrency-interest/2016-May/015125.html
