RFR JDK-8087113: Websocket API and implementation

[Anthony Vanelverdinghe]

Hi Pavel

Here are my suggestions concerning the public types:

java.net.http.WebSocket
- order the arguments of
static Builder newBuilder(URI uri, HttpClient client, Listener listener)
consistently with the 2-argument method:
static Builder newBuilder(URI uri, Listener listener, HttpClient client)

- remove CompletableFuture<Void> sendText(Stream<? extends CharSequence> 
message):
there are many candidate convenience methods, and the choice for 
Stream<? extends CharSequence> seems arbitrary. For example, why this 
one, and not methods with char[] (analog to the byte[] method for 
sendBinary), Iterable<? extends CharSequence> (as used by 
java.nio.file.Files::write), Reader, ...
convenience methods can always be added in a later version, based on 
empirical evidence of which convenience methods would add most value

- remove CompletableFuture<Void> sendBinary(byte[] message, boolean isLast):
same motivation as the previous one

- add CompletableFuture<Void> sendBinary(ByteBuffer message)
to be consistent with sendText, which has the single-parameter method 
sendText(CharSequence message)

- CompletableFuture<Void> sendClose(CloseCode code, CharSequence reason)
change the type of "reason" to String

java.net.http.WebSocket.Builder
- Builder connectTimeout(long timeout, TimeUnit unit)
use a single java.time.Duration argument instead

java.net.http.WebSocket.Listener
- onText/onBinary/onPing/onPong
return an immediately-complete CompletionStage (i.e. 
CompletableFuture.completedStage(null)) instead of null
return CompletionStage<Void> instead of CompletionStage<?>

- onText
change the type of the "message" parameter with ByteBuffer, and specify 
it contains UTF-8 encoded bytes & has a backing array

java.net.http.WebSocket.Text
remove this interface. I believe the Text interface doesn't carry its 
weight. By specifying the Listener::onText method as above, one can 
easily do something like new String().getBytes(message.array(), UTF_8) 
to get a CharSequence

java.net.http.WebSocketHandshakeException
- extend IOException, consistently with all other exception classes in 
java.net and javax.net
- if HttpResponse can indeed be null (as is documented in getResponse), 
the constructor's current implementation will throw a 
NullPointerException in this case

Kind regards,
Anthony

On 25/03/2016 17:21, Pavel Rappo wrote:
> Hi,
>
> Could you please review my change for JDK-8087113
>
>     http://cr.openjdk.java.net/~prappo/8087113/webrev.03/
>
> This webrev contains initial implementation and basic tests for WebSocket API.
> Specification conformance & interoperability testing has been performed with
> Autobahn Testsuite [1]. As for API tests, I will provide more of them later.
>
> Thanks,
> -Pavel
>
> --------------------------------------------------------------------------------
> [1] http://autobahn.ws/testsuite/
>
>