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

Hmm CDI supposes the container is running so a container being a CDI is weird IMO - even if I get the big shortcut you want to introduce which makes sense? +1 to hide CDIContainerLoader 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:33 GMT+01:00 Jozef Hartinger <jharting(a)redhat.com&gt;: > I am wondering if we need to expose both CDIContainer and > CDIContainerLoader to the user. Could we instead follow what CDI does where > CDI is the only class exposed and CDIProvider is a not-exposed SPI? Also, > given that CDIProvider is kind of an SPI what about reusing it as a > provider of CDIContainer also? > > Another though: What about changing the signature to something like: > > public class CDIContainer extends CDI<Object> implements AutoCloseable > > Jozef > > > On 03/01/2015 03:13 PM, John D. Ament wrote: > > 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&gt; > 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&gt; 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&gt; 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://rmannibucau.wordpress.com >>>> https://github.com/rmannibucau >>>> >>>> >>>> 2015-02-27 8:05 GMT+01:00 Jozef Hartinger <jharting(a)redhat.com&gt;: >>>> > 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&gt; 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(a)redhat.com&gt;: >>>> >>>> >>>> >>>> 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&gt; >>>> >>>> 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&gt; >>>> >>>>> 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 >>>> >>>>>> 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. >>>> >> >>>> >> >>>> > >>>> >>> > > _______________________________________________ > cdi-dev mailing listcdi-dev@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. > > > > _______________________________________________ > 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. >