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/
