[WEBSOCKET_SPEC-163] ServerContainer (multiple issues) Created: 06/Mar/13  Updated: 07/Mar/13  Resolved: 07/Mar/13

Status: Resolved
Project: websocket-spec
Component/s: None
Affects Version/s: 1.0
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Pavel Bucek Assignee: dannycoward
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Tags: proposedfinaldraft

 Description   
  • ServerContainer should NOT extend WebSocketContainer
  • ServerContainer methods should NOT throw DeploymentException - we already have a mechanism which validates deployed endpoints, but this is being executed during (Filter) initialization phase. Having DeploymentException here will force us to do this twice. Deployment phase (as defined in ServerContainer) seems more like collecting set of classes which will be taken into consideration during deployment, not anything else.


 Comments   
Comment by markt_asf [ 06/Mar/13 ]

Re: ServerContainer should NOT extend WebSocketContainer
Why?
I don't object to this change but I'd like to understand why you want to change it. It seems natural to me.

ServerContainer methods should NOT throw DeploymentException
This I do disagree with. Just because you opt to make a note of the request and deploy later - other implementations are free to make different design decisions. Further, a requested enhancement (not in 1.0) was to be able to deploy at any point in time. When that feature is added (or if containers opt to permit it now) these methods will need to throw a DeploymentException.

Comment by markt_asf [ 06/Mar/13 ]

One additional issue is how to resolve the following:

In a purely programmatic deployment on a Servlet container (no annotations, no SCIs) the implementation will need a reference to the ServletContext in order to configure a Servlet/Filter/Something to handle WebSocket connections.

There is currently no container/implementation neutral way of providing the ServerContainer with its associated ServletContext. I'd prefer to have a neutral way of doing this rather then having to have something container and/or implementation specific.

Comment by Pavel Bucek [ 06/Mar/13 ]

ad 1)

container is not yet created at that point and this class represents it. Additionally I don't see any point of having possibility to call connectToServer methods during application deployment.

ad 2)

I don't agree with "other implementations are free to make different design decision".

ServerContainer javadoc:

/**
 * The ServerContainer is the specialized view of the WebSocketContainer available
 * in server-side deployments. There is one ServerContainer instance per
 * websocket application. The ServerContainer holds the methods to be able to
 * register server endpoints during the initialization phase of the application.
 * For example, for websocket enabled web containers, the registration methods
 * may be called to register server endpoints from a ServletContextListener 
 * during the deployment of the WAR file containing the endpoint. Once the 
 * application deployment phase is complete, and the websocket application has
 * begun accepting incoming connections, the registration methods may no
 * longer be called.
 * 
 * @author dannycoward 
 */

I can see your point and I was considering to go with two checks of deployed classes/configs, but it just does not make much sense to me if I'm considering ServerContainer as it is currently defined..

Comment by jitu [ 06/Mar/13 ]

>"There is currently no container/implementation neutral way
> of providing the ServerContainer with its associated
> ServletContext. I'd prefer to have a neutral way of doing
> this rather then having to have something container and/or > implementation specific."

Pehaps, it is better to pass the context in the API method
ServerContainerProvider.getServerContainer(Object ctxt). Then a developer may pass ServletContext in servlet containers to create websocket server container.

Comment by markt_asf [ 06/Mar/13 ]

ServerContainerProvider.getServerContainer(Object ctxt) works for me.

Comment by dannycoward [ 07/Mar/13 ]

We will retain the ServerContainer as a subtype of WebSocketContainer. Programmatic deployment of server endpoints is a container level function specific to server implementations.

Programmatic deployment will require some of the static validity checking of the endpoint to take place at the time of call of the API.

For now, we may need to rely on container specific support to be able to associate the ServerContainer instance with the ServletContext. We haven't made it a goal (yet!) in this project to have the implementations be totally portable across web containers. I'll add this to the list for .next, when we have more implementations to look at to see where there are container specific hooks.

Mark, we will be interested to see how you implement this bootstrap at application deployment time, and will be happy to compare notes when we have it working properly.

I have filed a separate issue (165) on the bootstrap/portability issue so that we can decide, when we have some more implementation experience ourselves and from others, as to how best to address this issue.

Generated at Mon Jul 25 14:07:44 UTC 2016 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.