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

[Romain Manni-Bucau]

@Jozef: you are right, this observer can solve most of the issues, just
need a sample in the spec IMO


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 10:36 GMT+01:00 Romain Manni-Bucau <rmannibucau at gmail.com>:

> Hmm CDI supposes the container is running so a container being a CDI is
> weird IMO - even if I get the big shortcut you want to introduce which
> makes sense?
>
> +1 to hide CDIContainerLoader
>
>
> 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 10:33 GMT+01:00 Jozef Hartinger <jharting at redhat.com>:
>
>>  I am wondering if we need to expose both CDIContainer and
>> CDIContainerLoader to the user. Could we instead follow what CDI does where
>> CDI is the only class exposed and CDIProvider is a not-exposed SPI? Also,
>> given that CDIProvider is kind of an SPI what about reusing it as a
>> provider of CDIContainer also?
>>
>> Another though: What about changing the signature to something like:
>>
>> public class CDIContainer extends CDI<Object> implements AutoCloseable
>>
>> Jozef
>>
>>
>> On 03/01/2015 03:13 PM, John D. Ament wrote:
>>
>> So, I think I've gathered enough feedback at this point, and seen some of
>> the API changes.  I'll hopefully be including some doc changes this week,
>> but one question - do we want to start the SE specific stuff as its own
>> asciidoc file?
>>
>>  Changes made:
>>
>>  - Changed return value to CDI<Object> to provide better capability out
>> of the box.
>> - Added AutoCloseable to CDIContainer, provided default implementation of
>> calling shutdown.
>> - Added synchronization support to the method body that retrieves the
>> singleton instance (BTW, I'm not sure if this is even useful TBH as each
>> impl, including the RI, needs to provide this class in its own format).
>> - Made the params map typed to <String,Object>
>>
>>  @Romain Your case isn't really supportable yet, until we have static
>> injection support.  You'd still have to have a managed version of Runner to
>> work against.
>>
>>  John
>>
>> On Sat, Feb 28, 2015 at 4:11 PM Romain Manni-Bucau <rmannibucau at gmail.com>
>> wrote:
>>
>>> Yes but not the way to get an instance. Even Unmanaged does it.
>>>
>>> What can be awesome is to have static inject for it:
>>>
>>> public class Runner {
>>>
>>> @Inject
>>> private static MyMain main;
>>>
>>> public static void main(String[] arg) {
>>>     try (CDIContainer c = CDIContainer.newContainer()) {
>>>         main.doWork();
>>>     }
>>> }
>>>
>>> }
>>>
>>> And not a single additional line :).
>>>  Le 28 févr. 2015 19:05, "John D. Ament" <john.d.ament at gmail.com> a
>>> écrit :
>>>
>>>  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.
>>>>> >>
>>>>> >>
>>>>> >
>>>>>
>>>>
>>
>> _______________________________________________
>> cdi-dev mailing listcdi-dev at lists.jboss.orghttps://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/20150304/c1a158dc/attachment-0001.html