You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2021/04/23 07:33:10 UTC

[Bug 65262] New: Enable websocket endpoints to be IoC friendly (javaee integration at least)

https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

            Bug ID: 65262
           Summary: Enable websocket endpoints to be IoC friendly (javaee
                    integration at least)
           Product: Tomcat 10
           Version: unspecified
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: WebSocket
          Assignee: dev@tomcat.apache.org
          Reporter: rmannibucau@gmail.com
  Target Milestone: ------

Follow up of the dev list discussion.
Goal of the ticket is to use the InstanceManager to create websocket endpoints,
encoders, decoders in default configurators (server for sure and would be great
in client when possible - not sure it is that trivial since it is designed to
be standalone).
Note that except the wiring of the instance manager it also implies the proper
destroy call wiring too.

Side note on client side: if the wiring is too complicated cause you don't
always know if a server is running or not - so instance manager access can be
hard, it would be neat to enable to have a SPI to change the default
configurator implementation, goal is the same as on client side: enable to
respect 7.1.1 (javaee integration) spec part which says that endpoints are CDI
aware (in practise I guess it will do a CDI lookup to respect all CDI features
and not reimplement them).

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

--- Comment #6 from Mark Thomas <ma...@apache.org> ---
@Rémy
I think I can see a way to do that. We'll need to check which Configurator was
used in the WsSession constructor to make sure we don't call the
InstanceManager twice. It does mean that the timing of the call to
InstanceManager will vary depending on the Configurator but I don't think that
should be an issue.

@Romain
Do you need the InstanceManager to used used when testing the validity of the
encoder/decoder classes (e.g.
https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/websocket/Util.java#L345)
or just when an Encoder/Decoder instance is created for an Endpoint?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

--- Comment #5 from Remy Maucherat <re...@apache.org> ---
(In reply to romain.manni-bucau from comment #4)
> @Mark: this issue is about the default configurator, fully agree when a
> custom configurator is used tomcat will not care.

I agree if using the default configurator then we know what it does and instead
of complying 100% with the specification, we could shortcut to the instance
manager to create the instance and solve your problem. There are shortcuts for
IO as well, so why not.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

--- Comment #1 from Mark Thomas <ma...@apache.org> ---
WebSocket endpoints already use the InstanceManager.

https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/websocket/WsSession.java#L180

https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/websocket/WsSession.java#L548

Is the request to have the InstanceManager create the endpoint instances from
the class name rather than use InstanceManager.newInstance(Object) ?


Using the InstanceManager for Encoders and Decoders looks to be rather more
complex. I looked at the spec but I didn't see a requirement for this 7.1.1
only discusses WebSocket endpoint classes.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

--- Comment #2 from romain.manni-bucau <rm...@gmail.com> ---
Hmm, endpoint api starts from a class on server side so should use the related
instance manager instantiator and not only the injection "newInstance"
probably. For an annotated endpoint you likely inject PojoEndpoint since the
bean is not an endpoint which is not that useful.

I'd also like encoders/decoders to comply to the IoC/instance manager otherwise
no way to get a bound resource like a mapper.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

--- Comment #3 from Mark Thomas <ma...@apache.org> ---
Section 3.1.7 of the WebSocket specification requires endpoint instances are
created via ServerEndpointConfig.Configurator.getEndpointInstance(). Users are
free to supply their own Configurator implementation. Therefore, it is not
possible to use the InstanceManager to create the endpoint. The current call to 
InstanceManager.newInstance(Object) is the best that can be achieved.

I'll spend a little more time looking at the encoders and decoders but, given
that there is no specification requirement for these to support DI, whether
anything gets implemented is going to depend a lot on how cleanly it can be
done.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Mark Thomas <ma...@apache.org> ---
Fixed in:
- 10.0.x for 10.0.6 onwards
- 9.0.x for 9.0.46 onwards
- 8.5.x for 8.5.66 onwards

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

--- Comment #4 from romain.manni-bucau <rm...@gmail.com> ---
@Mark: this issue is about the default configurator, fully agree when a custom
configurator is used tomcat will not care.
I also agree encoders/decoders IoC support is not in the specification but not
having it lead to very weird patterns - even for just a plain JSON codec when
you want to do it right and consistent with app code/config, makes it not
natural at all and lead to easy leaks so I think it is worth going through the
InstanceManager anyway and make it to the spec at some point.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

--- Comment #8 from Mark Thomas <ma...@apache.org> ---
I've applied a fix for Endpoints to 10.0.x, 9.0.x and 8.5.x. I'll look at
Encoders and Decoders next so if there are any issues with the current approach
do let me know.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 65262] Enable websocket endpoints to be IoC friendly (javaee integration at least)

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65262

--- Comment #7 from romain.manni-bucau <rm...@gmail.com> ---
@Mark functionally I can leave with current validation but theorically the
validation is only known of the IoC but it is not super aligned on the spec.
To illustrate it take a CDI or Spring encoder, it can use constructor
injections and not have a default no-arg constrauctor but spec requires it so I
guess validation can be let this way and we can ask the spec to drop this later
moving the validation to the instantiation time only.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org