Skip to main content

[jsr356-experts] Re: [jsr356-users] Re: Pot Pourri of issues from Public Draft

  • From: Justin Lee < >
  • To: Danny Coward < >
  • Cc:
  • Subject: [jsr356-experts] Re: [jsr356-users] Re: Pot Pourri of issues from Public Draft
  • Date: Sun, 27 Jan 2013 00:09:57 -0500

Yeah, I'm an idiot on this.  The whole annotation thing completely slipped past me.  Of *course* it has be the class def.  please disregard.  and forget.  ;)

The IllegalStateException works for me.  Largely on checked vs runtime grounds but your other reasoning sounds good, too.


On Fri, Jan 25, 2013 at 6:23 PM, Danny Coward < " target="_blank"> > wrote:
Hi Justin,

Thanks for looking these over - couple things inline...


On 1/19/13 8:39 AM, Justin Lee wrote:



On Fri, Jan 18, 2013 at 8:23 PM, Danny Coward < " title="[GMCP] Compose a new mail to " rel="noreferrer" target="_blank"> > wrote:
Hi folks,

We have many issues to resolve, so I picked a collection to get things started. As usual, I have outlined an approach for each issue to keep things moving, so if you have other options or suggestions to add, please chime in.

* Consider requiring authentication schemes to be supported on the client container
http://java.net/jira/browse/WEBSOCKET_SPEC-75

I think BASIC and DIGEST are probably the most common. I think this is a good idea, but I am wary of trying to come up with this in a short period of time for the first version. Given that developers can intercept and modify the handshake request (see below) and inspect the handshake response, they have the ability to build http-based authentication schemes on top of the API without any changes. So I'd suggest we defer requiring client containers to support any particular authentication schemes until the next release.

Agreed.  The hybi list spents weeks on this without any meaningful resolution that I could see.  No need to rush it when existing solutions will work.
 


* Allow annotated client endpoints to intercept handshake (as well as programmatic ones)
http://java.net/jira/browse/WEBSOCKET_SPEC-111

Currently we have a scheme whereby programmatic client endpoint can override the default configuration (for example, to intercept the opening handshake). But there isn't a way to allow annotated client endpoints to do the same thing. Since server endpoints of both kinds can do this, and given developers may want to plug in authentication schemes or other header-manipulating code into client POJOs, I think we should allow this by (based on the current scheme) adding an optional configuration() attribute to @WebSocketClient.

Would it be a class parameter passed to the annotation?  My fear about passing in class references and having the framework instantiate them is that it precludes the use of injection frameworks like guice/CDI to wire up those dependencies without some gymnastics inside the class's initializers to find the injection context and force that wiring explicitly.

We are using classes for various developer-provided things in the API - especially in the annotations - configuration on @WebSocketEndpoint, and of course the Encoder and Decoder classes.

When you talk about use of injection frameworks, I think what we allow for is the websocket implementation to delegate instance creation to other frameworks. For example, in the EE setting, to delegate endpoint instance creation to CDI so that EE things can be injected into endpoints. Is this what you had in mind ?

But using classes for a variety of things: configuration/encoders/decoders/applicationconfig we do preclude the application developer from creating instances of those things themselves, using their own favorite framework. I don't know how we'd do that, since we can't reference instances of things from annotations.

 


* Allow Java primitives and class equivalents as message parameters for @WebSocketMessage methods.

http://java.net/jira/browse/WEBSOCKET_SPEC-96

Currently, you are allowed to annotate methods that offer a String, ByteBuffer, byte[] or custom developer object (with decoder) parameter for whole message processing or (String,boolean), (ByteBuffer, boolean) and (byte[], boolean) parameters for partial message processing. The request is to allow methods that offer Java primitives for message processing.

Since all of the classes Byte, Short, Integer, Long, Float, Double, Boolean have unambiguous mappings from String via their single argument String constructors, I think this would be a nice addition to allow this for processing text messages. Character has one string parameter constructor, but of course String.toCharArray() is unambiguous enough for strings of length 1, so that could be included for completeness. Any failure to decode an incoming message to its primitive/class equivalent, according to those mapping would generate a DecodeException, in exactly the same way as happens for incoming messages that the runtime is trying to map to a custom developer object.

I don't see an unambiguous way to map binary data to Java primitive types, so I'm suggesting we make this work only for text messages.


* Lifecycle of Decoders/Encoders
http://java.net/jira/browse/WEBSOCKET_SPEC-99

Currently the specification leaves it open to implementations as to when they instantiate Encoders and Decoders for incoming and outgoing messages. We could leave this up to the implementations, but it would be better to define it since they are supposed to be portable. There are clearly a number of choices here, but I do think we should guarantee that no single instance of an Encoder or Decoder object is shared between different connections. What are other people's thoughts ?


See my comments above about passing classes around.  I'm ok making that requirement clear in the docs.  
 

* Calling methods on closed Sessions / Making Session Closeable
http://java.net/jira/browse/WEBSOCKET_SPEC-104
http://java.net/jira/browse/WEBSOCKET_SPEC-87

Once a websocket session has closed, it is possible that code will still reference the instance and (inadvertently?) call methods on it. In such situations, I think the methods should throw IllegalStateException because the session is closed and its data and operations are now meaningless.

The JDK throws IOException in that case (or some subclass).  We might consider following that example for consistency's sake.

For for this case, my thinking was that once the Session is closed developers really should not even be still using it. The same session can't be reopened, and the container probably wants to recycle it or forget it. So I was looking for an appropriate runtime exception. If we used a checked exception, it would be saying its reasonable to write code that calls sessions after they are invalid, and that developers always have to code with an error handing block in case. Whereas, they just shouldn't even get here.

IllegalStateException seemed to fit that bill (http://docs.oracle.com/javase/7/docs/api/java/lang/IllegalStateException.html), and is also consistent with what HttpSession does after its been invalidated.

 

Secondly, since Session already has a close() method, we could make it implement java.io.Closable, and hence it would then be possible to use it in a JDK 7 try-with-resources block. I think this is a good idea too.


big +1

Yes, and I think this is one of those things that 'shows well' too !

- Danny



 

* Extension parameter ordering
http://java.net/jira/browse/WEBSOCKET_SPEC-105

Currently the API models extension parameters as Map<String, String>, yet the parameters have an order in the http headers. While this order doesn't have a meaning in the websocket protocol specification, both the MUX and Compression extensions attach semantic meaning to specific parameters, so it is perfectly possible that some new extension might attach semantic meaning to their order. So, I think our API needs to preserve them. I suggest we model extension parameters as List<Parameter> where Parameter is a member type of Extension, and contains the parameter name and value as Strings.


I don't think header ordering matters anywhere in any common protocols.  I don't think we should bend over backwards making this a requirement outside of something forcing us to.  I'd be really, really surprised if any extensions make it out of specification with that requirement.  Even HTTP only specifies the first 2ish headings and that's merely to aid in protocol identification.  I'm a -1 on this one.


* Number of instances of client container / server container
http://java.net/jira/browse/WEBSOCKET_SPEC-119
http://java.net/jira/browse/WEBSOCKET_SPEC-120

Currently these are both modeled using the class WebSocketContainer. This relates to two requests:

1) To allow the client developer to instantiate client container instances at will, so that he can use different instances with different property values, such as async send timeout or maximum message sizes. This seems like a reasonable thing to accommodate.

Absolutely.  Consider a proxying websocket application trying to talk to two different servers for whatever reason.
 

2) To use a one instance of the server container WebSocketContainer per application per VM instead of the current one instance per VM for all applications. I think this is reasonable too - exactly matches the cardinality of the ServletContext for web components.

+1 
 



--
Danny Coward
Java EE
Oracle Corporation



--


--
Danny Coward
Java EE
Oracle Corporation



--


[jsr356-experts] Pot Pourri of issues from Public Draft

Danny Coward 01/19/2013

[jsr356-experts] Re: Pot Pourri of issues from Public Draft

Justin Lee 01/19/2013

[jsr356-experts] Re: Pot Pourri of issues from Public Draft

Mark Thomas 01/21/2013

[jsr356-experts] Re: [jsr356-users] Re: Pot Pourri of issues from Public Draft

Danny Coward 01/25/2013

[jsr356-experts] Re: [jsr356-users] Re: Pot Pourri of issues from Public Draft

Justin Lee 01/26/2013

[jsr356-experts] Re: [jsr356-users] Re: Pot Pourri of issues from Public Draft

Danny Coward 01/25/2013

[jsr356-experts] Re: [jsr356-users] Re: Pot Pourri of issues from Public Draft

Justin Lee 01/27/2013
 
 
Close
loading
Please Confirm
Close