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

@Antoine: to boot multiple BM it would need to break contextuality of CDI, one way being to forbid any serialization (not a big constraint in SE). In this case container is bound to a bean manager which changes the API Jozef propose 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:49 GMT+01:00 Antoine Sabot-Durand <antoine(a)sabot-durand.net <mailto:antoine@sabot-durand.net>>: Hi John, I think it could be a good idea to write down all of these to have a more stable doc for discussion. You should update the google doc with the result of this discussion. I agree with the following points in this thread : - Minimize the number of new Class / Interface. CDI and CDIProvider usage is still not very clear for end user so we should add the strict minimum and try to enhance existing API / SPI when possible - Be able to bootstrap CDI without returning BeanManager (env if the API give possibility to access it if needed). End user don’t need that : CDI app can start with an observer for instance Something not dealt with but that we should keep in mind : - Providing a Java SE solution that could be easily used for a servlet bootstrap of CDI. I don’t know if we’ll standardize this but we definitely should keep this use case in mind and my bonus, it’s out of scope but I didn’t see anytime there that prevent this nice to have: - support the possibility to boot multiple BeanManager in Java SE. Antoine > Le 1 mars 2015 à 15:13, John D. Ament <john.d.ament(a)gmail.com > <mailto:john.d.ament@gmail.com>> a écrit : > > 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(a)gmail.com <mailto:rmannibucau@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(a)gmail.com <mailto:john.d.ament@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(a)gmail.com <mailto:rmannibucau@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://www.tomitribe.com/> > http://rmannibucau.wordpress.com > <http://rmannibucau.wordpress.com/> > https://github.com/rmannibucau > > > 2015-02-27 8:05 GMT+01:00 Jozef Hartinger > <jharting(a)redhat.com <mailto:jharting@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(a)redhat.com <mailto:jharting@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://www.tomitribe.com/> > >>> http://rmannibucau.wordpress.com > <http://rmannibucau.wordpress.com/> > >>> https://github.com/rmannibucau > >>> > >>> > >>> 2015-02-26 8:32 GMT+01:00 Jozef Hartinger > <jharting(a)redhat.com <mailto:jharting@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(a)redhat.com <mailto:jharting@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(a)redhat.com > <mailto:jharting@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/2c362161e18dd521f8e83c27151ddad46... > >>>>>> > >>>>>> Let me know your thoughts. > >>>>>> > >>>>>> Thanks, > >>>>>> > >>>>>> John > >>>>>> > >>>>>> > >>>>>> _______________________________________________ > >>>>>> cdi-dev mailing list > >>>>>> cdi-dev(a)lists.jboss.org > <mailto:cdi-dev@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(a)lists.jboss.org > <mailto:cdi-dev@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(a)lists.jboss.org <mailto:cdi-dev@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(a)lists.jboss.org <mailto:cdi-dev@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(a)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.