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

Your definition seems to fit the standalone need: libraries = not CDI based code (case of a standalone), the runner class (Mymain) doesnt have to be a CDI bean but has to get injections to launch the CDI code. 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 13:59 GMT+01:00 Jozef Hartinger <jharting(a)redhat.com <mailto:jharting@redhat.com>>: UnmanagedInstance is provided to make it easier for libraries to perform dependency injection on classes that are for some reason not CDI beans. It should not be a substitute for lookup of CDI beans. Therefore, I do not see UnmanagedInstance fitting here. On 03/04/2015 01:47 PM, Romain Manni-Bucau wrote: > Hmm > > I think one of the main point I try to push is we have a bunch of > API to do it already, if we need yet another API to do the same > we have several choices: > - we love creating APIs > - all previous APIs are failures and should be deprecated or fixed > - there is a full mismatch with embedded and EE case (but we have > existing proofs it is not the case) > > I think we should help user to not be lost between all APIs and I > strongly believe we can't do anything on container to lookup > beans (EJBContainer#getContext was a try which is close to it but > it actually just limited user experience compared to existing > solutions). > > What's the issue with UnmanagedInstance? > > > > 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 13:43 GMT+01:00 Jozef Hartinger <jharting(a)redhat.com > <mailto:jharting@redhat.com>>: > > The only argument I found supporting a strict separation of > those two APIs is that it makes it easier to control when a > user should or should not use boot (i.e. it should not be > used in EE for example). > > That's a good argument. It's not however necessarily only > achieved by two separate interfaces but can be as well be > achieved with a subclass, e.g: > - CDI for runtime operations only > - StartedCDI extends CDI (or CDIContainer or whatever - the > name does not matter at this point) for runtime operations + > shutdown. > > Normally, CDI is available only. The boot API however would > return StartedCDI thus allowing a user to shutdown what they > started. > > > > On 03/04/2015 12:24 PM, John D. Ament wrote: >> This is actually based on what we discussed in one of the EG >> meetings >> >> http://transcripts.jboss.org/meeting/irc.freenode.org/cdi-dev/2015/cdi-de... >> >> John >> >> On Wed, Mar 4, 2015 at 4:05 AM Jozef Hartinger >> <jharting(a)redhat.com <mailto:jharting@redhat.com>> wrote: >> >> Well it's nowhere given that we must have two separate >> interfaces for this. We can combine the start/stop API >> with the existing one to provide an application with a >> single reference representing the CDI container. >> >> >> On 02/28/2015 07:05 PM, John D. Ament wrote: >>> 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://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://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. >>> >> >>> >> >>> > >>> >> > >