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

[John D. Ament]

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.
- 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.


>
>
>  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 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 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.
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/cdi-dev/attachments/20150225/2485b906/attachment.html