Norman Maurer wrote:
Hey Stuart,
thanks for start the thread. I just finished the review of your changes
and have a few comments.
* FrameHandler:
I don't know if I like the fact that the same interface is used for
"assembled Fragments" (when
WebSocketSession.setAutomaticallyAssembleFragments(true) was used) and
not assembled Fragments (when
WebSocketSession.setAutomaticallyAssembleFragments(false) was used". I
think we really should either use another interface or do the assemble
work in an abstract implementation of the FrameHandler and just provide
abstract methods to implement that act on the "assembled Frame" then.
Also with the current interface design it is up to the user to detect
when the Frame is complete by check if he received all of the data. I
think we should make it easier and just tell the user when it is complete.
Using an abstract frame handler to do the assembly sounds like a better
idea.
Also I wonder if it is really useful to also get notified with the frame
content for other frames then Binary and Text. My idea would be that we:
- automatically respond to a Ping with a Pong
- automatically echo back the Close Frame and after that close the
connection
- Ignore Pong frames
This is probably ok, there is however some info that I think we need to
provide:
- So way of taking the idle time on the connection, so if the client has
sent a ping the application has some way of knowing that the client is
still alive.
- When the close frame is received the application should receive some
kind of notification with any data that was in the close frame.
- Pong frames should definitely not be ignored, if the application has
decided to send a ping frame they would want to be notified of the result.
The fact why I think we should just do this is that otherwise it is easy
for the user to make the whole thing messed up and so not be conform
with the RFC anymore. Do you think there are real use-cases for get the
content of those in the high-level API ? If not we may not want to
expose this at all.
* TextFrameSender / BinaryFrameSender
What about add an extra method to it which takes a FileChannel, offset,
length and SendCallback as parameter. So we could make use of zero copy
if a user want to send a file content in a frame.
Something like this:
void sendBinary(FileChannel, long offset, long length, SendCallback
callback);
Sounds reasonable.
* WebSocketFrame
Should be renamed to WebSocketFrameHeader or something like this if we
keep it like it is atm.
+1
Stuart
---
Norman Maurer
nmaurer(a)redhat.com <mailto:nmaurer@redhat.com>
JBoss, by Red Hat
Am 25.01.2013 um 02:25 schrieb Stuart Douglas <sdouglas(a)redhat.com
<mailto:sdouglas@redhat.com>>:
> Hi guys,
>
> Last night Norman and I had a bit of a talk about what the high level
> Websocket API should look like, and Norman came up with some
> interfaces as a prototype.
>
> So after looking at Norman's prototype I had a bit more of a think
> about what our high level API should look like and this is what I have
> come up with:
>
>
https://github.com/stuartwdouglas/undertow/compare/websockets_high_level_api
>
> I have been thinking about the use cases that we want to address with
> the high level API, and when you would want to use the high level API
> vs the low level XNIO API.
>
> My thinking is that in general if you want to use async IO and deal
> with message bytes as soon as they arrive this is covered quite nicely
> by our existing low level API. The high level API should focus on
> async operations involving complete messages, and blocking operations.
>
> The JSR 356 implementation will just be implemented as a fairly thin
> wrapper around our high level API.
>
> Sending:
>
> This is handled by (Text|Binary)FrameSender.
>
> I think there are a few different use cases for the high level API in
> terms of sending messages:
>
> - Complete message async, with a callback on completion
> - Complete message blocking
> - Complete message blocking with Stream / Writer
> - Fragmented versions of all of the above.
>
> There is one use case that is not covered here, and that is sending a
> partial messaged using async IO, as mentioned above the reason why I
> don't think we should have this as part of the high level API is
> because this is essentially what is provided by our low level API.
>
> Receiving:
>
> In terms of receiving I think that we should just have a single
> handler type:
>
>
https://github.com/stuartwdouglas/undertow/compare/websockets_high_level_...
>
> This just receives complete messages. I think this is preferable to
> having to register 5 different types of handler for every frame type.
>
> These handlers just receive complete messages that are fully buffered.
>
> In terms of fragmented messages I think we should provide 2 ways of
> dealing with them, and have a setting that controls which one is in use:
>
>
https://github.com/stuartwdouglas/undertow/compare/websockets_high_level_...
>
> If this is true then we will just assemble the fragmented messaged
> automatically and deliver it as 1 logical message. Otherwise they will
> just be delivered as they arrive.
>
> I would have liked to provide a way to receive messages via an
> InputStream / Reader, to provide a way to get around fully buffering
> messaged when using blocking IO, however I can't really think of a
> nice way to do this.
>
> What do you guys think? I know there is still some stuff missing (e.g.
> we need a way to propagate auth and session information from the
> original HTTP request).
>
> Stuart
>