jsr356-experts@websocket-spec.java.net

[jsr356-experts] Re: [jsr356-users] Re: Feedback pls ! Re: connectToServer

From: Danny Coward <danny.coward_at_oracle.com>
Date: Fri, 01 Mar 2013 15:58:05 -0800

Ok thanks everyone. I've added this in for v014.

- Danny

On 3/1/13 4:57 AM, Mark Thomas wrote:
> Joe Walnes<joe_at_walnes.com> wrote:
>
>> B.
> +1
>
> Mark
>
>> On Thu, Feb 28, 2013 at 5:49 PM, Danny Coward
>> <danny.coward_at_oracle.com>wrote:
>>
>>> Hi folks,
>>>
>>> So that we can close out this last issue, I would just like a little
>> more
>>> feedback. So let me try to lay this out as a simple choice. (Scott
>> pls
>>> correct anything you think I'm not representing correctly!)
>>>
>>> connectToServer currently has two forms, one with the annotated
>> class, the
>>> other with the ClientEndpointConfig instance.
>>>
>>> These forms allow the spec to require (in the EE setting) that the
>>> implementation use CDI to instantiate the endpoints, so the client
>>> endpoints are automatically non-contextual managed beans, and so,
>> without
>>> any extra work on the part of the developer, they have the sort of
>> nice
>>> injectable properties that many of the other Java EE components have
>> out of
>>> the box.
>>>
>>> However, for developers who don't care about this, and need to share
>> state
>>> between the websocket application and the client endpoint instance,
>> its
>>> clunky, as they'd have to use the
>> ClientEndpointConfig.getUserProperties()
>>> property bag for programmatic endpoints, or do something funkier with
>> a
>>> custom Configurator for annotated endpoints. Which argues for the
>> form of
>>> connectToServer that takes endpoint instances created by the
>> developer, not
>>> the implementation.
>>>
>>> I feel that supporting the first form is very important, and would
>> like to
>>> support the second form. The only hesitation I have is the extra
>> complexity
>>> of more API and having to explain the difference. For comparison, the
>>> Servlet API has both per instance and per class registration, so if
>> we add
>>> these methods, we're in ok company :)
>>>
>>> I guess another reason for adding the instance based approach to what
>> we
>>> have is for developers who have their own injection frameworks. Is
>> that
>>> compelling to folks here as a reason ?
>>>
>>> So,
>>>
>>> A) Keep the API as is, don't add the per -instance connectToServer
>> methods.
>>> or
>>>
>>> B) Pls add the new methods, they're useful and people can read the
>> javadoc.
>>> Thanks !
>>>
>>> - Danny
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On 2/27/13 6:17 PM, Scott Ferguson wrote:
>>>
>>> On 2/27/13 5:49 PM, Danny Coward wrote:
>>>
>>> Hi Scott,
>>>
>>> My apologies, I didn't mean to imply that developers can't use the
>> CDI
>>> APIs themselves to create endpoints that have all the nice injectable
>>> properties we want, or to misrepresent what you are asking for in the
>>> specification.
>>>
>>> I suppose I'm trying to find out if folks would like the
>> implementation to
>>> do this for them for client endpoints, as it does in the EE setting
>> for
>>> server endpoints.
>>>
>>> Or ask developers to use the CDI APIs themselves if they want the
>>> injectable properties.
>>>
>>> Or offer both modes.
>>>
>>>
>>> thanks for the clarification.
>>>
>>> -- Scott
>>>
>>>
>>> - Danny
>>>
>>> On 2/27/13 5:30 PM, Scott Ferguson wrote:
>>>
>>> On 2/27/13 4:47 PM, Danny Coward wrote:
>>>
>>> My apologies to Scott to have left it so late to respond to this
>> change.
>>> In part I was waiting until we had cleaned up the
>>> *Configuration/Configurator API, which I think we have now, as of
>> v013.
>>> Here is today's API:-
>>>
>>> WebSocketContainer {
>>> Session connectToServer(Class<?> annotatedEndpointClass, URI path)
>>> throws DeploymentException, IOException;
>>> Session connectToServer(Class<? extends Endpoint> endpointClass,
>>> ClientEndpointConfiguration cec, URI path) throws
>> DeploymentException,
>>> IOException;
>>> }
>>>
>>> I think since this came up, we have embraced the CDI instantiation
>> when
>>> this API is in a Java EE container. This obviously brings the
>> benefits of
>>> being able to inject EJBs and CDI managed beans into websocket
>> endpoints,
>>> the bread and butter for Java EE developers, and which forces a model
>> where
>>> the implementation is doing the instantiation.
>>>
>>>
>>> No.
>>>
>>> It's just false that CDI "forces a model where the implementation is
>> doing
>>> the instantiation" on the client side.
>>>
>>> A client in a Java EE container is fully capable of instantiating its
>> own
>>> CDI-injected endpoint before calling connectToServer, either with
>> @Inject
>>> Endpoint, or @Inject Instance<Endpoint> or from the BeanManager
>>> directly, or even from basic encapsulation of its own injection
>> points. It
>>> already has access to EJBs and everything.
>>>
>>> The client already has all the features you list.
>>>
>>> Besides, if you do want ClientWebSocketContainer to instantiate under
>> CDI,
>>> you really should be using:
>>>
>>> BeanManager.getBeans(Type beanType, Annotation... qualifiers)
>>>
>>> Please note the qualifiers. If you're calling with just the Class<?>,
>> and
>>> stripping the qualifiers, you're using CDI incorrectly, (and thereby
>> taking
>>> power away from the client). The API really needs to be:
>>>
>>> connectToServer(URI path, Type endpointType, Annotation...
>> qualifiers);
>>> We have also added the concept of allowing Configurators to control
>>> endpoint instance creation on the server side.
>>>
>>> So if we still feel this is a relevant use case, my thought is we
>> could
>>> add the analogous factory method to the ClientEndpointConfigurator.
>>>
>>>
>>> Clients don't need factories.
>>>
>>> On the server side, that factory is needed because server endpoints
>> are
>>> created on demand as requests come in. On servers, new endpoints are
>> not
>>> under the server's control. It's a reactive model and requires some
>> kind of
>>> factory.
>>>
>>> On a client, the client knows when it wants to create an endpoint.
>> That's
>>> the whole point of connectToServer. That's what it does. There's no
>> need
>>> for any reactive API.
>>>
>>> Client and server are not symmetrical in this case.
>>>
>>>
>>> Or we could simply add the deploy by endpoint instance methods Scott
>> has
>>> proposed. (we still need the deploy by class/config for the EE
>> folks).
>>> Please explain why you think EE doesn't have CDI already available.
>>>
>>> -- Scott
>>>
>>>
>>> What is the group feeling ?
>>>
>>> - Danny
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On 1/14/13 8:32 AM, Scott Ferguson wrote:
>>>
>>> The new connectToServer is difficult to use.
>>>
>>> Instead of a Class parameter, it should be an object. The container
>>> shouldn't need to instantiate the object, and the Class model makes
>> it
>>> difficult to connect between the client application and the client
>>> endpoint. (Because any shared state/object needs to be passed through
>> a
>>> custom ClientEndpointConfiguration.)
>>>
>>> The API should be simplified to:
>>>
>>> WebSocketContainer {
>>> Session connectToServer(Object endpoint, URI path);
>>>
>>> Session connectToServer(Object endpoint, URI path,
>>> ClientEndpointConfiguration cec);
>>> }
>>>
>>> If the "endpoint" object is already of type Endpoint, it's used
>> directly.
>>> Otherwise, a skeleton/proxy is created, i.e. annotation stuff.
>>>
>>> There's no need for separate methods for Endpoint vs annotation.
>>>
>>> -- Scott
>>>
>>>
>>>
>>> --
>>> <http://www.oracle.com> * Danny Coward *
>>> Java EE
>>> Oracle Corporation
>>>
>>>
>>>
>>>
>>> --
>>> <http://www.oracle.com> * Danny Coward *
>>> Java EE
>>> Oracle Corporation
>>>
>>>
>>>
>>>
>>> --
>>> <http://www.oracle.com> * Danny Coward *
>>> Java EE
>>> Oracle Corporation
>>>


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