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