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

Thu Feb 26 10:19:10 EST 2015

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.