WebSocket client API

[Roger Riggs]

Hi Pavel,

On 9/2/2015 4:40 PM, Pavel Rappo wrote:
> Hi Roger, thanks for looking at this!
>
>> On 1 Sep 2015, at 17:05, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>>
>> - The Incoming class combines separate functions that would be easier to
>>   use if the methods were directly on the Builder.
> ...
>
>> |WebSocket ws = WebSocket.newBuilder("ws://websocket.example.com") .onReceiveText( (ws, text) -> System.out.println(text)) .onError( (ws, t) -> t.printStackTrace()) .onClose( (ws, msg) -> System.out.println("closing: " + msg))  .create();|
> At some stage earlier I had a prototype that looked exactly what you've described
> here.
>
> One thing that caused me to discard it, was not lambda expressions themselves,
> but the fact that we're splitting one type into several fine-grained ones. Some
> simple examples showed me that non-trivial (useful) handlers would need to hold
> and share some state. With split interfaces (an interface per function) we don't
> have the kind of convenience and simplicity we would have with a single
> interface.
>
> Thus we have to organize this state/data sharing somewhere outside, making the
> code a tiny bit more tangled and rigid (subjectively).
>
> With functional interfaces and lambdas I feel it looks cleaner, but it might be
> more painful to code against. I understand I might be overestimating the amount
> of inconvenience here, so I would appreciate to hear your opinion on this.
A few points:
  - I see it the other way, there are multiple functions being combined 
into a single type.
  - By making this a single class, the API forces the application to 
subclass and commits the concrete supertype.
    This is generally accepted as being undesirable.
  - The application may very well want to have different code and 
structures for binary vs text data
    and to code them in different classes.

>
>>   The Builder methods can use generic java.util.function interfaces with the appropriate parameter types.
>>   Each of the callbacks should include the websocket,
> Yes, I thought about it. Among other good things, it gives us most of the
> required types for free (i.e. no need to create many tiny types).
>
> The thing that I didn't like was that if we choose to go with anonymous classes,
> we would be stuck with these overly generic method names: apply, accept, etc.
> That might become awkward if we're going to use the same type but from different
> perspectives.
The nice thing about functional interfaces (single method) is that they 
are matched
on the structure of the signature, not on the names.  For Lambdas, the 
names don't matter.
With lambdas available, I doubt a developer would use anonymous classes 
anymore,
though if the developer wants create their own explicit classes the 
generic method names apply.
(as you point out below)

>
> For example, both read and write back pressure have essentially the same
> interface and the same semantics. It's the same message. The difference is
> whether one is receiving or sending. In this case -- which side of the
> callback one is on. Either one is calling .suspend(boolean) or one is being
> called as suspend(boolean).
>
> (for illustrative purposes only):
>
> /**
> * A control used to communicate back-pressure to a peer.
> */
> public interface Backpressure {
>
>     /**
>      * Tells peer to suspend/resume data flow.
>      *
>      * @param f {@code true} to suspend, {@code false} to resume
>      */
>     void suspend(boolean f);
> }
>
> Then WebSocket.Builder could have this:
>
> /**
> * Configures the builder with a user-provided back-pressure handler.
> *
> * <p> {@code WebSocket} will notify the user on whether they should
> * suspend sending by calling {@code control.suspend(true)}, and {@code
> * control.suspend(false)} when sending can be resumed.
> *
> * @param handler a back pressure handler
> * @return this builder
> */
> public Builder sendingBackpressure(Backpressure handler) {
>     return this;
> }
>
> And WebSocket could have this:
>
> /**
> * Returns a back pressure control, a user can use to notify a server
> * it should suspend/resume sending.
> *
> * @return a control to communicate an incoming back-pressure.
> */
> public abstract Backpressure receivingBackpressure();
>
> This type is already lambda-freindly (it's a functional interface). Now let's
> change it to Consumer<Boolean>, just to not create an additional type.
>
> Then in the builder you can easily hide Consumer's "accept" by providing a
> method reference:
>
> WebSocket ws = WebSocket.newBuilder("ws://websocket.example.com")
>         .sendingBackpressure(this::onHold())
>
> or lambda
>
> WebSocket ws = WebSocket.newBuilder("ws://websocket.example.com")
>         .sendingBackpressure(() -> onHold())
>
> But not in the WebSocket:
>
> 	ws.receivingBackpressure().consume(true);
>
> Generic functional interface is a good handler (callback), but a terrible domain
> object.
I see your point but I don't think its an appropriate re-use of the 
Backpressure interface.
The ws.receivingBackpressure method would have a boolean argument (t/f) 
to tell the WebSocket
how to behave.  The current WebSocket API is not structured to have a 
'controls' style of API.

BTW, the suspendReceiving() and resumeReceiving() methods would be 
better as a single
method with a boolean argument.  If the application has some state 
itself about suspended state,
it will need to use if (suspended) resumeRecieving() else 
suspendReceiving() to get the desired effect.

>
>> - The design to wrap the Outgoing data creates an extra step that makes the API
>>   a bit harder to use.  It would be more natural to add the sendAsync  methods to WebSocket
>>   and avoid the creation of extra objects.
>>
>> |ws.sendAsync("hello"); ws.sendAsync(bytebuffer); // an alternate signature could use varargs, ByteBuffer data... |
> I considered it some time ago. Here's the obvious disadvantage of not having
> this step I've found: it mixes the way data is sent with the way data is
> constructed. Previous prototypes had several different "send" methods:
>
> 	S = {send, sendAsync, queue,...}
>
> We also have several different ways (4 at the moment) to construct data:
>
> 	D = {ByteBuffer, CharSequence, Iterator<CharSequence>, Iterator<ByteBuffer>, ...}
>
> So if we put this into a single type we'll have to provide {S x D} methods:
>
> send(ByteBuffer);
> send(CharSequence);
> send(Iterator<CharSequence>);
> send(Iterator<ByteBuffer>);
> ...
> queue(Iterator<ByteBuffer>);
>
> Agreed, at the moment we have only 1 send and just 4 types of data sources. But
> size S or D might change in future, and we'd better have a prepared way to adapt
> to it.

I'm less worried about the number of methods, if the methods are named 
logically, the developer
will find the right method by completion in the IDE or know what they are.
I'm more concerned about extra steps the developer has to write code for 
that
seem unnecessary compared to a straightforward implementation of the 
function.

Thanks, Roger

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20150903/1f7f6f2c/attachment.html>