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

John D. Ament john.d.ament at gmail.com
Sat Feb 28 13:05:18 EST 2015 

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>
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>:
> > 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> 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>:
> >>>>
> >>>> 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>
> >>>> 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>
> >>>>> 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
> >>>>>> 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
> >>>> 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/20150228/1d6bc283/attachment-0001.html

More information about the cdi-dev mailing list