[jsr356-experts] Re: Updated Spec: v005: Close to EDR

From: Danny Coward <danny.coward_at_oracle.com>
Date: Fri, 21 Sep 2012 17:26:15 -0700

Hi Jeanfrancois,

Thanks for the detailed review... I've addressed most of them in the
current draft, and filed an issue on a couple of them - also a couple
comments - inline:-

On 9/20/12 3:45 PM, Jeanfrancois Arcand wrote:
> Salut,
> some comments on the specification (catching up, so ignore what have
> already been discussed in the list :-))
> * Section 3.1.5 "In particular, they may wish to intervene more in
> the handshake process. For example,
> they may wish to use Http cookies to track clients, or insert
> application specific headers
> in the handshake response"
> o The Javascript API doesn't allow to read the response's header
> after an hanshake. So I'm sure sure allowing an application to
> do that is the right way to go.
One of the use cases was session tracking..adding perhaps an app
specific cookie to the handshake response...

> * Section 4.2: "The decorated method must either have no parameters
> or have a single parameter that is a Session object."
> o Why only Session? Although the JavaScript API isn't allowing
> to set any custom header, this is still possible to pass query
> strings to the URL. Hence it would be convenient to be able to
> see the Request information from an onOpen method.
> + we should support getQueryString/getParams etc. on the
> Session API
ok fair enough, I added these.

> +
> * Section 4.2.1 : "The decorated method must either have no
> parameters or have a single parameter that is a Session object."
> o getCloseReason on Session doesn't looks corect. What will be
> the returned value when the session is active? I think the
> CloseReason should be passed to the @WebSocketClose method
> instead of being intermixed with the session API.
Yes I agree that's better. Changed in EDR.

> o
> Javadocs
> * Any reason to use RemoteEndpoint instead of WebSocket as the
> class's name? I found it confusing, e.g we don't call a
> RemoteSocket a Socket in java.io ;-)
See below. Would like to do a naming/style overhaul a bit further down
the road.
> * Should the HanshakeRequest by typed so getSession() returns the
> type instead of Object? So we don't have to call instanceof to
> make sure we are getting the right Session object at runtime.
We were trying to find a way not to make a hard dependency on the
servlet API. Its definitely not ideal as it is.
> *
> * Endpoint.onClose should pass the close closing code IMO so an
> application can acts the same way the JavaScript API allow, do
> something based on a close code. Live above, I don't think mixing
> close status inside the Session is the right way to go.
> * ClientContainer.getActiveSessions(): This needs to be clarified as
> Session may become invalid at any moment.
yes updated.
> * Do we really need the Decoder.willDecode(..) API? Should we throw
> an exception instead when the decode gets called?
The idea was to allow the Decoder to pass up the opportunity to another
decoder that can manage the same time. The exception throw would mean a
failure and the container would not try to find another decoder. But
perhaps you're right and we should combine those two ideas into the
exception ? Or better, add a flag to the exception to indicate whether
to try the next suitable decoder ? Better for lambdas too.
> *
> * Session.getMessageHandler() must return a Set<MessageHandler>
> In general all the APIs aren't fluid and never return itself ... for
> example, the Session interface could return itself instead of 'void'
> all over the place. Would be nice to be able to chain calls
> mySession.addEncoder(...).addEncoder(...)
> That's cosmetic, I know.
Yes I agree. What I'd like to do is do a fuller more general API
usability rework sometime before public draft for fluidity and naming. I
filed a JIRA issue to track it.

- Danny
> -- Jeanfrancois
> On 12-09-14 7:44 PM, Danny Coward wrote:
>> Hi folks,
>> I've updated the specification document and API again this week. I'm
>> still looking for review comments. This week only some smaller
>> changes, the API tweaks arising from our work on the reference
>> implementation (see below).
>> The v005 API is in the source repository, under the v005 branch.
>> http://java.net/projects/websocket-spec/sources
>> spec document and javadoc here:-
>> http://java.net/projects/websocket-spec/downloads/directory/Spec%20javadoc%20Drafts/v005
>> Changes
>> - added to the security chapter in the specification.
>> - bolstered up some of the javadoc here and there, esp annotations
>> - added missing constructors on DecodeException
>> - add WebSocketError annotation, mirroring other lifecycle annotations.
>> We have had a number of drafts and revisions now, and I think with
>> one more round of review next week and comment we are almost ready
>> for Expert Draft review. Many of you will know all about this stage,
>> but the main point is that it allows us to get review from a wider
>> audience. Not everything has to be finished, or complete. But I think
>> we have a lot of our content in place, and with another round of
>> review, it will be in good enough shape for new readers to give us
>> good feedback on what is there.
>> We have made the RI project public now: its at
>> http://java.net/projects/tyrus/ if you want to take a look. Still a
>> work in progress, but it is tracking the v003 API.
>> Cheers,
>> - Danny
>> --
>> <http://www.oracle.com> *Danny Coward *
>> Java EE
>> Oracle Corporation

<http://www.oracle.com> 	*Danny Coward *
Java EE
Oracle Corporation