Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: None
    • Labels:
      None

      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.

        Activity

        Hide
        dannycoward added a comment -

        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.

        Show
        dannycoward added a comment - 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.
        Hide
        markt_asf added a comment -

        ServerContainerProvider.getServerContainer(Object ctxt) works for me.

        Show
        markt_asf added a comment - ServerContainerProvider.getServerContainer(Object ctxt) works for me.
        Hide
        jitu added a comment -

        >"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.

        Show
        jitu added a comment - >"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.
        Hide
        Pavel Bucek added a comment -

        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..

        Show
        Pavel Bucek added a comment - 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..
        Hide
        markt_asf added a comment -

        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.

        Show
        markt_asf added a comment - 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.
        Hide
        markt_asf added a comment -

        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.

        Show
        markt_asf added a comment - 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.

          People

          • Assignee:
            dannycoward
            Reporter:
            Pavel Bucek
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: