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

Jozef Hartinger jharting at redhat.com
Thu Feb 26 02:32:48 EST 2015


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 
> <mailto: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 <mailto: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  <mailto: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/bd600171/attachment-0001.html 


More information about the cdi-dev mailing list