[cdi-dev] Feedback - CDI bootstrap API (CDI-26)

Jozef Hartinger jharting at redhat.com
Wed Mar 4 07:59:39 EST 2015


UnmanagedInstance is provided to make it easier for libraries to perform 
dependency injection on classes that are for some reason not CDI beans. 
It should not be a substitute for lookup of CDI beans. Therefore, I do 
not see UnmanagedInstance fitting here.

On 03/04/2015 01:47 PM, Romain Manni-Bucau wrote:
> Hmm
>
> I think one of the main point I try to push is we have a bunch of API 
> to do it already, if we need yet another API to do the same we have 
> several choices:
> - we love creating APIs
> - all previous APIs are failures and should be deprecated or fixed
> - there is a full mismatch with embedded and EE case (but we have 
> existing proofs it is not the case)
>
> I think we should help user to not be lost between all APIs and I 
> strongly believe we can't do anything on container to lookup beans 
> (EJBContainer#getContext was a try which is close to it but it 
> actually just limited user experience compared to existing solutions).
>
> What's the issue with UnmanagedInstance?
>
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog 
> <http://rmannibucau.wordpress.com> | Github 
> <https://github.com/rmannibucau> | LinkedIn 
> <https://www.linkedin.com/in/rmannibucau> | Tomitriber 
> <http://www.tomitribe.com>
>
> 2015-03-04 13:43 GMT+01:00 Jozef Hartinger <jharting at redhat.com 
> <mailto:jharting at redhat.com>>:
>
>     The only argument I found supporting a strict separation of those
>     two APIs is that it makes it easier to control when a user should
>     or should not use boot (i.e. it should not be used in EE for example).
>
>     That's a good argument. It's not however necessarily only achieved
>     by two separate interfaces but can be as well be achieved with a
>     subclass, e.g:
>     - CDI for runtime operations only
>     - StartedCDI extends CDI (or CDIContainer or whatever - the name
>     does not matter at this point) for runtime operations + shutdown.
>
>     Normally, CDI is available only. The boot API however would return
>     StartedCDI thus allowing a user to shutdown what they started.
>
>
>
>     On 03/04/2015 12:24 PM, John D. Ament wrote:
>>     This is actually based on what we discussed in one of the EG
>>     meetings
>>
>>     http://transcripts.jboss.org/meeting/irc.freenode.org/cdi-dev/2015/cdi-dev.2015-01-14-17.04.log.html
>>
>>     John
>>
>>     On Wed, Mar 4, 2015 at 4:05 AM Jozef Hartinger
>>     <jharting at redhat.com <mailto:jharting at redhat.com>> wrote:
>>
>>         Well it's nowhere given that we must have two separate
>>         interfaces for this. We can combine the start/stop API with
>>         the existing one to provide an application with a single
>>         reference representing the CDI container.
>>
>>
>>         On 02/28/2015 07:05 PM, John D. Ament wrote:
>>>         Maybe I'm misreading, but I don't see us adding another API
>>>         to do the same thing here - we're introducing new
>>>         functionality.
>>>
>>>         CDIContainer/Loader on startup/shutdown of the application
>>>
>>>         CDI for runtime usage within the application to interact
>>>         with the container.
>>>
>>>         John
>>>
>>>         On Fri, Feb 27, 2015 at 3:40 AM Romain Manni-Bucau
>>>         <rmannibucau at gmail.com <mailto:rmannibucau at gmail.com>> wrote:
>>>
>>>             sure I fully agree excepted I think introducing yet
>>>             another API to do
>>>             the same thing is not good so super tempting to skip it
>>>             and wait for
>>>             feedbacks rather than introducing it eagerly.
>>>
>>>
>>>             Romain Manni-Bucau
>>>             @rmannibucau
>>>             http://www.tomitribe.com
>>>             http://rmannibucau.wordpress.com
>>>             https://github.com/rmannibucau
>>>
>>>
>>>             2015-02-27 8:05 GMT+01:00 Jozef Hartinger
>>>             <jharting at redhat.com <mailto:jharting at redhat.com>>:
>>>             > My point is that from the application perspective, the
>>>             user obtains one
>>>             > container handle for eventual shutdown (CDIContainer)
>>>             and then looks up a
>>>             > different container handle (CDI) that they can use for
>>>             real work (lookup /
>>>             > event dispatch / etc.) It would be cleaner if the
>>>             container gave away a
>>>             > single handle that can do all of that.
>>>             >
>>>             >
>>>             > On 02/26/2015 05:42 PM, Romain Manni-Bucau wrote:
>>>             >
>>>             > Not sure I get how a CDI instance can help.
>>>             >
>>>             > But container.getBeanManager() sounds nice is not a
>>>             shortcut for
>>>             > CDI.current().getBm() otherwise it looks like
>>>             duplication to me.
>>>             >
>>>             > Can we make container not contextual - dont think so?
>>>             If so it makes sense
>>>             > otherwise I fear it doesnt add much.
>>>             >
>>>             > Le 26 févr. 2015 16:19, "Jozef Hartinger"
>>>             <jharting at redhat.com <mailto:jharting at redhat.com>> a écrit :
>>>             >>
>>>             >> I like the initialize + close() combination and the
>>>             try-with-resources
>>>             >> usage.
>>>             >> What looks weird to me is that at line one you obtain
>>>             a container handle:
>>>             >>
>>>             >> try (CDIContainer container =
>>>             CDIContainer.newCDIContai...
>>>             >> CDI.current().getBeanManager() ...
>>>             >>
>>>             >> and then at line two you call a static method to
>>>             perform a container
>>>             >> lookup :-/
>>>             >>
>>>             >> An API that allows you to use the container handle
>>>             you already got is way
>>>             >> better IMO, e.g.:
>>>             >>
>>>             >> try (CDIContainer container =
>>>             CDIContainer.newCDIContai...
>>>             >> container.getBeanManager()
>>>             >>
>>>             >> If CDIContainer.newCDIContainer() returns an CDI
>>>             instance or its subclass,
>>>             >> we get this easily.
>>>             >>
>>>             >> On 02/26/2015 08:58 AM, Romain Manni-Bucau wrote:
>>>             >>>
>>>             >>> Hi guys
>>>             >>>
>>>             >>> why note keeping it simple?
>>>             >>>
>>>             >>> try (CDIContainer container =
>>>             CDIContainer.newCDIContainer(/* optional
>>>             >>> map to configure vendor features */)) {
>>>             >>>  CDI.current().getBeanManager()....
>>>             >>> }
>>>             >>>
>>>             >>> Not sure the point having initialize() + having
>>>             shutdown = close
>>>             >>> really makes the API more fluent and modern IMO.
>>>             >>>
>>>             >>> Also to be fully SE I guess provider() method would
>>>             be needed even if
>>>             >>> optional (SPI usage by default):
>>>             >>>
>>>             >>> try (CDIContainer container =
>>>             >>>
>>>             >>>
>>>             CDIContainer.provider("org.jboss.weld.WeldCdiContainerProvider").newCDIContainer())
>>>             >>> {
>>>             >>>  CDI.current().getBeanManager()....
>>>             >>> }
>>>             >>>
>>>             >>> Finally I think having a kind of getInstance
>>>             shortcut could be a plus for
>>>             >>> SE:
>>>             >>>
>>>             >>> try (CDIContainer container =
>>>             CDIContainer.newCDIContainer()) {
>>>             >>>  container.newInstance(MyAppRunner.class /* optional
>>>             qualifiers */
>>>             >>> ).run(args);
>>>             >>> }
>>>             >>>
>>>             >>> Using container to get an instance would create the
>>>             instance and bind
>>>             >>> it to the container lifecycle (mainly for
>>>             predestroy) avoiding this
>>>             >>> boilerplate code in all main which will surely only
>>>             be used to launch
>>>             >>> a soft.
>>>             >>>
>>>             >>> wdyt?
>>>             >>>
>>>             >>>
>>>             >>>
>>>             >>> Romain Manni-Bucau
>>>             >>> @rmannibucau
>>>             >>> http://www.tomitribe.com
>>>             >>> http://rmannibucau.wordpress.com
>>>             >>> https://github.com/rmannibucau
>>>             >>>
>>>             >>>
>>>             >>> 2015-02-26 8:32 GMT+01:00 Jozef Hartinger
>>>             <jharting at redhat.com <mailto:jharting at redhat.com>>:
>>>             >>>>
>>>             >>>> Comments inline
>>>             >>>>
>>>             >>>> On 02/25/2015 05:53 PM, John D. Ament wrote:
>>>             >>>>
>>>             >>>> Sorry Jozef, your email fell into the pits of
>>>             google inbox's "smart
>>>             >>>> sorting"
>>>             >>>> features.
>>>             >>>>
>>>             >>>> On Thu, Feb 12, 2015 at 3:18 AM Jozef Hartinger
>>>             <jharting at redhat.com <mailto:jharting at redhat.com>>
>>>             >>>> wrote:
>>>             >>>>>
>>>             >>>>> Hi John, comments inline:
>>>             >>>>>
>>>             >>>>>
>>>             >>>>> On 02/11/2015 06:02 PM, John D. Ament wrote:
>>>             >>>>>
>>>             >>>>> Jozef,
>>>             >>>>>
>>>             >>>>> Most of what you see there is taken from the
>>>             original doc, since
>>>             >>>>> everyone
>>>             >>>>> seemed to be in agreement.  I think the map is
>>>             just a safeguard in case
>>>             >>>>> of
>>>             >>>>> additional boot options available in some
>>>             implementations (e.g. I think
>>>             >>>>> OWB/OpenEJB have some options.. currently OpenEJB
>>>             supports an embedded
>>>             >>>>> CDI
>>>             >>>>> boot mode).
>>>             >>>>>
>>>             >>>>> No, I am fine with the map. What I am questioning
>>>             is the type of the
>>>             >>>>> map.
>>>             >>>>> Usually, data structures with a similar purpose
>>>             use Strings as their
>>>             >>>>> keys.
>>>             >>>>> This applies to ServletContext attributes,
>>>             InvocationContext data,
>>>             >>>>> Servlet
>>>             >>>>> request/session attributes and others. I am
>>>             therefore wondering whether
>>>             >>>>> there is a usecase for the proposed unbound key
>>>             signature or not.
>>>             >>>>
>>>             >>>>
>>>             >>>> I think that's more of a placeholder, I was
>>>             assuming it would be
>>>             >>>> Map<String,Object> once we clarify everything.
>>>             >>>>
>>>             >>>>>
>>>             >>>>>
>>>             >>>>> We spoke a few times about BeanManager vs CDI. 
>>>             BeanManager was
>>>             >>>>> preferable
>>>             >>>>> since there's no easy way to get the the instance,
>>>             CDI is easier to get
>>>             >>>>> and
>>>             >>>>> more aligned with how you would get it.  Usually
>>>             people expect the
>>>             >>>>> BeanManager to be injected or available via JNDI,
>>>             neither would be the
>>>             >>>>> case
>>>             >>>>> here.
>>>             >>>>>
>>>             >>>>> If CDI 2.0 targets Java SE then this container
>>>             initialization API will
>>>             >>>>> become something that ordinary application
>>>             developers use to start/stop
>>>             >>>>> CDI
>>>             >>>>> in their applications. It therefore cannot be
>>>             considered an SPI but
>>>             >>>>> instead
>>>             >>>>> should be something easy to use. On the other
>>>             hand, BeanManager is
>>>             >>>>> definitely an SPI. It is used in extension,
>>>             frameworks and generally
>>>             >>>>> for
>>>             >>>>> integration. Not much by applications directly.
>>>             Therefore, I don't see
>>>             >>>>> how
>>>             >>>>> the container bootstrap API and BeanManager fit
>>>             together. IMO the
>>>             >>>>> bootstrap
>>>             >>>>> API should expose something that makes common
>>>             tasks (obtaining a
>>>             >>>>> contextual
>>>             >>>>> reference and firing and event) easy, which the
>>>             CDI class does.
>>>             >>>>>
>>>             >>>>> Plus do not forget that BeanManager can be
>>>             obtained easily using
>>>             >>>>> CDI.getBeanManager().
>>>             >>>>
>>>             >>>>
>>>             >>>> I'm not disagreeing.  There's a few things I'd
>>>             consider:
>>>             >>>>
>>>             >>>> - Is this mostly for new apps or existing?  If
>>>             existing, it's probably
>>>             >>>> using
>>>             >>>> some internal API, if new it can use whatever API
>>>             we give.
>>>             >>>> - I don't want to return void, we should give some
>>>             kind of reference
>>>             >>>> into
>>>             >>>> the container when we're done booting.
>>>             >>>>
>>>             >>>> Agreed, we should not be returning void.
>>>             >>>>
>>>             >>>> - CDI is a one step retrievable reference, where as
>>>             BeanManager is a two
>>>             >>>> step reference.  With that said, BeanManager makes
>>>             more sense to return
>>>             >>>> here.  Another thought could be we invent some new
>>>             class that has both,
>>>             >>>> but
>>>             >>>> that's really redundant.
>>>             >>>>
>>>             >>>> Why do you think BeanManager makes more sense here?
>>>             Especially given the
>>>             >>>> assumption that application code is going to call
>>>             this init/shutdown
>>>             >>>> API, I
>>>             >>>> don't see BeanManager as making more sense.
>>>             >>>>
>>>             >>>>
>>>             >>>>>
>>>             >>>>>
>>>             >>>>> Yes, this is the container start API. Sounds like
>>>             you have some good
>>>             >>>>> ideas for things like XML configuration or
>>>             programmatic configuration,
>>>             >>>>> both
>>>             >>>>> of which are being tracked under separate
>>>             tickets.  One idea might be
>>>             >>>>> for an
>>>             >>>>> optional param in the map to control packages to
>>>             scan/ignore, in that
>>>             >>>>> map.
>>>             >>>>>
>>>             >>>>> I am wondering whether this configuration should
>>>             be something optional
>>>             >>>>> built on top of the bootstrap API or whether we
>>>             should consider making
>>>             >>>>> it
>>>             >>>>> mandatory. Either way, we cannot add the bootstrap
>>>             API to the spec
>>>             >>>>> without
>>>             >>>>> explicitly defining how it behaves. My implicit
>>>             assumption of the
>>>             >>>>> proposal
>>>             >>>>> is that the container is supposed to scan the
>>>             entire classpath for
>>>             >>>>> explicit
>>>             >>>>> or implicit bean archives (including e.g. rt.jar),
>>>             discover beans, fire
>>>             >>>>> extensions, etc. This worries me as this default
>>>             behavior is far from
>>>             >>>>> being
>>>             >>>>> lightweight, which CDI for Java SE initially aimed
>>>             to be.
>>>             >>>>
>>>             >>>>
>>>             >>>> Yes, the spec must be updated to reflect the
>>>             behavior of SE mode.  I
>>>             >>>> plan to
>>>             >>>> get that completely into the google doc before
>>>             opening any spec changes
>>>             >>>> in a
>>>             >>>> PR.
>>>             >>>>
>>>             >>>>>
>>>             >>>>>
>>>             >>>>> We didn't want to over load the CDI interface.  It
>>>             already does a lot.
>>>             >>>>> This is really SPI code, CDI even though it's in
>>>             the spi package is
>>>             >>>>> used in
>>>             >>>>> a lot of application code.
>>>             >>>>>
>>>             >>>>> I would personally prefer to have it all in one
>>>             place. Having
>>>             >>>>> CDIContainer, CDIContainerLoader, CDI and
>>>             CDIProvider makes it more
>>>             >>>>> difficult to know when to use what.
>>>             >>>>
>>>             >>>>
>>>             >>>> The problem is that most CDI (the interface)
>>>             operations are against a
>>>             >>>> running container.  I think we spoke about
>>>             leveraging CDIProvider at one
>>>             >>>> point (in fact, I mistakenly called CDIContainer
>>>             CDIProvider not even
>>>             >>>> realizing it was there).  I doubt that most app
>>>             developers use it
>>>             >>>> currently,
>>>             >>>> there's not even a way to get a reference to it
>>>             that I'm aware of.  It's
>>>             >>>> used by the implementor only.
>>>             >>>>
>>>             >>>> I don't think there's a conflict. CDI class would
>>>             still only provide
>>>             >>>> methods
>>>             >>>> to be run against a running container. The
>>>             difference is that there
>>>             >>>> would be
>>>             >>>> additional static methods to get this running
>>>             container (CDI class) to
>>>             >>>> you
>>>             >>>> by starting the container.
>>>             >>>>
>>>             >>>> Either way, I agree that reusing CDIProvider is a
>>>             must. There is no
>>>             >>>> reason
>>>             >>>> to define a new class for the same purpose.
>>>             >>>>
>>>             >>>>
>>>             >>>> I expect that my changes in the CDI spec around
>>>             this will state, along
>>>             >>>> the
>>>             >>>> lines of:
>>>             >>>>
>>>             >>>> To retrieve a CDIContainer to launch, do this:
>>>             >>>>
>>>             >>>> CDIContainer container =
>>>             CDIContainerLocator.getCDIContainer();
>>>             >>>> container.initialize();
>>>             >>>> ... do work
>>>             >>>>
>>>             >>>> Once you want to shutdown the container, do this:
>>>             >>>>
>>>             >>>> container.shutdown();
>>>             >>>>
>>>             >>>> (we may want to consider implementing
>>>             AutoCloseable, an oversight on my
>>>             >>>> part)
>>>             >>>>
>>>             >>>> and then later on
>>>             >>>>
>>>             >>>> - What happens if I call CDIContainerLocator in an
>>>             app server
>>>             >>>>
>>>             >>>> - It throws an IllegalStateException.
>>>             >>>>
>>>             >>>> - The container provides no beans of type
>>>             CDIContainer, it is managed
>>>             >>>> outside of the CDI container.
>>>             >>>>
>>>             >>>>
>>>             >>>>>
>>>             >>>>>
>>>             >>>>> John
>>>             >>>>>
>>>             >>>>> On Wed Feb 11 2015 at 4:21:50 AM Jozef Hartinger
>>>             <jharting at redhat.com <mailto:jharting at redhat.com>>
>>>             >>>>> wrote:
>>>             >>>>>>
>>>             >>>>>> Hi John, some thoughts:
>>>             >>>>>>
>>>             >>>>>> - instead of using BeanManager it makes more
>>>             sense to me to return a
>>>             >>>>>> CDI
>>>             >>>>>> instance, which is a more user-friendly API (and
>>>             it also exposes
>>>             >>>>>> access to
>>>             >>>>>> BeanManager)
>>>             >>>>>> - is there a usecase for arbitrary keys of the
>>>             "params" map or is
>>>             >>>>>> Map<String, ?> sufficient?
>>>             >>>>>> - if we could move the shutdown() method from
>>>             CDIContainer to the
>>>             >>>>>> actual
>>>             >>>>>> container handle that we obtain from
>>>             initialize(), that would look
>>>             >>>>>> more
>>>             >>>>>> object-oriented
>>>             >>>>>> - what exactly is initialize() supposed to do? Is
>>>             it supposed to start
>>>             >>>>>> scanning the entire classpath for CDI beans? That
>>>             could be a problem
>>>             >>>>>> especially with spring-boot-like fat jars. I
>>>             think we need an API to
>>>             >>>>>> tell
>>>             >>>>>> the container which classes / packages to
>>>             consider. Something like
>>>             >>>>>> Guice's
>>>             >>>>>> binding API perhaps?
>>>             >>>>>>
>>>             >>>>>> - the proposal makes me wonder whether
>>>             retrofitting this functionality
>>>             >>>>>> to
>>>             >>>>>> the CDI class wouldn't be a better option. It
>>>             could look like:
>>>             >>>>>>
>>>             >>>>>> CDI container = CDI.initialize();
>>>             >>>>>> container.select(Foo.class).get();
>>>             >>>>>> container.shutdown();
>>>             >>>>>>
>>>             >>>>>> compare it to:
>>>             >>>>>>
>>>             >>>>>> CDIContainer container = CDIContainerLoader.
>>>             getCDIContainer();
>>>             >>>>>> BeanManager manager = container.initialize();
>>>             >>>>>> manager.getBeans(...);
>>>             >>>>>> container.shutdown(manager);
>>>             >>>>>>
>>>             >>>>>>
>>>             >>>>>> On 02/10/2015 06:58 PM, John D. Ament wrote:
>>>             >>>>>>
>>>             >>>>>> All,
>>>             >>>>>>
>>>             >>>>>> I have the updated API here, and wanted to
>>>             solicit any final feedback
>>>             >>>>>> before updating the google doc and spec pages.
>>>             >>>>>>
>>>             >>>>>>
>>>             >>>>>>
>>>             >>>>>>
>>>             https://github.com/johnament/cdi/commit/2c362161e18dd521f8e83c27151ddad467a1c01c
>>>             >>>>>>
>>>             >>>>>> Let me know your thoughts.
>>>             >>>>>>
>>>             >>>>>> Thanks,
>>>             >>>>>>
>>>             >>>>>> John
>>>             >>>>>>
>>>             >>>>>>
>>>             >>>>>> _______________________________________________
>>>             >>>>>> cdi-dev mailing list
>>>             >>>>>> cdi-dev at lists.jboss.org
>>>             <mailto:cdi-dev at lists.jboss.org>
>>>             >>>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>             >>>>>>
>>>             >>>>>> Note that for all code provided on this list, the
>>>             provider licenses
>>>             >>>>>> the
>>>             >>>>>> code under the Apache License, Version 2
>>>             >>>>>>
>>>             (http://www.apache.org/licenses/LICENSE-2.0.html). For
>>>             all other ideas
>>>             >>>>>> provided on this list, the provider waives all
>>>             patent and other
>>>             >>>>>> intellectual
>>>             >>>>>> property rights inherent in such information.
>>>             >>>>>>
>>>             >>>>>>
>>>             >>>>
>>>             >>>> _______________________________________________
>>>             >>>> cdi-dev mailing list
>>>             >>>> cdi-dev at lists.jboss.org
>>>             <mailto:cdi-dev at lists.jboss.org>
>>>             >>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>             >>>>
>>>             >>>> Note that for all code provided on this list, the
>>>             provider licenses the
>>>             >>>> code
>>>             >>>> under the Apache License, Version 2
>>>             >>>> (http://www.apache.org/licenses/LICENSE-2.0.html).
>>>             For all other ideas
>>>             >>>> provided on this list, the provider waives all
>>>             patent and other
>>>             >>>> intellectual
>>>             >>>> property rights inherent in such information.
>>>             >>
>>>             >>
>>>             >
>>>
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/cdi-dev/attachments/20150304/b3f2a562/attachment-0001.html 


More information about the cdi-dev mailing list