WebSocket client API

[Simone Bordet]

Hi,

On Tue, Oct 6, 2015 at 12:27 PM, Pavel Rappo <pavel.rappo at oracle.com> wrote:
> Hi,
>
> Here's an update on the WebSocket API. This iteration tries to address all
> issues have been discussed so far:
>
>    webrev: http://cr.openjdk.java.net/~prappo/8087113/webrev.01/
>    javadoc: http://cr.openjdk.java.net/~prappo/8087113/javadoc.01/

I agree with Paul that the context does not provide much benefit and
will simplify the API :)

What it is still missing is the fact that there is no specification
about the onXXX methods regarding the lifecycle of the parameters
passed in.

For example, this is going to surprise users (simple echo):

onBinary((ws, bytes, last) -> {
    ws.sendBinary(bytes, last, null, (_, failure) -> {});
}

It's not going to work because the send is async, and there is no
specification about who owns the ByteBuffer "bytes".
In my experience, it is bad to force applications to perform a copy
(not to mention that the copy could be really expensive, as WebSocket
frames could be large).
This would also lead to applications being forced to block waiting for
the send to complete, which defeats the goal of an async API (and may
take a long time).
A throw-away ByteBuffer is an alternative, but it becomes an allocation hotspot.
There are well known, proven, and widely available solution for this
particular problem; any works for me, but designing the API without is
IMHO a mistake, especially considering that this is going to be a
low-level API that manages frames and not messages.

The *API* should provide a callback to notify when the ByteBuffer has
been consumed.
The current proposal suffers from the fact that you want to use only
1-2 classes (Handler and BiHandler) for too many use cases.
Again I agree with Paul that this smells, and I would prefer a
Listener interface.

StatusCode makes little sense to me: it's a wrapper for int, and
nothing more. I would prefer to see primitive int in the signatures.

Method isClosed() does not convey enough information.
WebSocket has built-in in the protocol half-closes, so it should be
able to report this information, like Socket returns
isInputShutDown(), isOutputShutDown() and isOpen().
A simple enum would do.

I think it's enough for the builder to provide an async build() method only.

There is no specification for connect and idle timeouts. Are these
supposed to be configured in HttpClient ? Likewise for cookies and
headers ?

-- 
Simone Bordet
http://bordet.blogspot.com
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz