Hi, Overall i think this API seems to be compressing down nicely to a small number of classes/interfaces, but i still think there is some room for further simplifications.
WebSocket
I don’t see the need for a context parameter. Context can be obtained via
capturing. That reduces the requirement for a specific but generic BiHandler,
since a BiConsumer could be used instead. A null completion handler could be
allowed for those not interested in completion events.
My preferred alternative to the callback handler approach is to return a
Completable<WebSocket> that completes with the WebSocket instance or
exceptionally. That to me better fits with the notion of completion of a
specific event (just like that when async building a connected WebSocket), that
allows one to chain off events or even block if necessary. It also reduces the
need for a specific but generic functional interface. I suspect the common case
would be to ignore the CF that is returned.
WebSocket.Builder
The protocol of receiving events is spread out over multiple behavioural
parameters. Instead we could have a functional interface,
WebSocketMessageListener say, with defaults for all but the most common event,
which i guess is the receiving of text messages.
It’s easy to reintroduce the current behaviour as a separate layer if so
desired e.g. a simple message event builder, but it’s really awkward to do it
the other way around, which is a design smell to me.
The builder can then be reformulated as:
Builder(String uri) // A URI is the only required argument
using(HttpClient ) // Overrides any previous call
using(HttpRequest.Builder ) // Overrides any previous call
subprotocols(String , String ...)
listener(WebSocketMessageListener ) // Overrides any previous call
WebSocket build( )
CF<WebSocket> buildAsync()
The end result is WebSocket.BiHandler/Handler go away to be replaced with one
specific web socket message listener interface whose documentation describes
the receiving protocol and the threading/concurrency callback behaviour, and
overall there is a simplification in the set of methods and their signatures.
Paul.
> On 6 Oct 2015, at 12:27, Pavel Rappo <pavel.ra...@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/
>
> Main differences from the previous version:
>
> * Extension support has been postponed and remains an open issue [1]
> * WebSocket.Builder now also accepts HttpRequest.Builder (for providing
> custom opening handshake headers)
> * Outgoing is gone, instead a user can send incomplete Binary and Text
> messages with ByteBuffers and CharSequences directly
> * Incoming is gone, instead WebSocket.Builder provides a handler
> assigning method per event (message or error)
> * Async methods take a custom context object and a potentially reusable
> completion handler (NIO2 style)
> * The API is now j.u.c.Flow-friendly
>
> -------------------------------------------------------------------------------
> [1] https://bugs.openjdk.java.net/browse/JDK-8138949;
> That doesn't mean the default implementation won't support
> 'permessage-deflate'.
>
> -Pavel
>
>> On 31 Aug 2015, at 15:30, Pavel Rappo <pavel.ra...@oracle.com> wrote:
>>
>> Hi,
>>
>> I would appreciate if you help to review a WebSocket client API proposed for
>> JDK 9. This work is a part of JEP-110 [1] and has started its public path
>> with
>> HTTP client review in March this year [2].
>>
>> Proposed WebSocket API is relatively small and focuses on convenient
>> exchange of
>> data in a fully asynchronous fashion. API consists of 6 types located in the
>> java.net package [3]:
>>
>> 1. WebSocket
>> 2. WebSocket.Builder
>> 3. WebSocket.Incoming
>> 4. WebSocket.Incoming.Chunks
>> 5. WebSocket.Outgoing
>> 6. WebSocketException
>>
>> Starting point is a class description for java.net.WebSocket. Along with
>> in-javadoc examples, several API test samples are provided in the webrev [4]
>> and
>> named test/java/net/WebSocket/Example%.java. They are only for informational
>> purposes and won't be included in the final version of the API.
>>
>> I would appreciate any feedback on this API. Thanks.
>>
>> -------------------------------------------------------------------------------
>> [1] http://openjdk.java.net/jeps/110
>> [2] http://mail.openjdk.java.net/pipermail/net-dev/2015-March/008932.html
>> [3] http://cr.openjdk.java.net/~prappo/8087113/javadoc.00/
>> [4] http://cr.openjdk.java.net/~prappo/8087113/webrev.00/
>>
>> -Pavel
>
signature.asc
Description: Message signed with OpenPGP using GPGMail
