[cdi-dev] Feedback - CDI bootstrap API (CDI-26)
Romain Manni-Bucau
rmannibucau at gmail.com
Thu Feb 26 11:42:59 EST 2015
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/2c362161e18dd521f8e83c27151dda
>>>>> d467a1c01c
>>>>>
>>>>> 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/20150226/dc02ff7a/attachment-0001.html
More information about the cdi-dev
mailing list