Danny,
thanks for the update and here is some initial feedback
I like the break down of all the different message handling interfaces -
makes it clear all the different options available. However it is not
clear if the annotation WebSocketMessage is able to represent all those
different signatures - ideally anything that can be done with an interface
can also be done with an annotation. Perhaps the annotation should just
say that it supports any method signatures of the MessageHandler
subtypes? For example I'd like to be able to do:
@WebSocketEndPoint
public class MyPojoWebSocket
{
@WebSocketMessage
public void myAsyncTextHandler(String part, boolean last)
{ ... }
}
Also for ease of use, I'd really like to see the API offer message
aggregation as part of the API. With annotations this could be:
@WebSocketEndPoint(maxMessage=8192)
public class MyPojoWebSocket
{
@WebSocketMessage
public void myAsyncTextHandler(String part)
{ ... }
}
You might need and extra interface for this and a setter in
EndpointConfiguration
For the send side of things, I firstly think SendHandler#onResult should
really be called SendCallback#onResult, or at least SendHandler#onResult
Also I'm a bit dubious about the send methods that take both a completion
handler AND return a future.
Typically one or the other style is used, so if you are using callbacks, it
is a bit wasteful to have to always create a future even if it was never
used. Currently you have the pair of methods:
void *sendBytes*(byte[] data)
java.util.concurrent.Future<SendResult> *sendBytes*(byte[] data, SendHandler
completion)
with the first being blocking. I think it would be better to have:
java.util.concurrent.Future<SendResult> *sendBytes*(byte[] data)
void *sendBytes*(byte[] data, SendHandler completion)
with neither being blocking. The user simply needs to call
sendBytes(data).get() to achieve blocking.
Alternately if you want to keep the blocking call, then don't have a future
in the API at all. Instead provide a utility implementation of
SendHandler that is a Future and can be used like:
SendHandlerFuture future=new SendHandlerFuture();
wsEndPoint.sendBytes(data,future);
future.get(10,TimeUnit.SECONDS);
This mantra could easily be wrapped up in a static method to make it a one
liner:
SendHandlerFuture.sendBytes(wsEndPoint,data).get(10,TimeUnit.SECONDS);
cheers
--
Greg Wilkins <gregw_at_intalio.com>
http://www.webtide.com
Developer advice and support from the Jetty & CometD experts.