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

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&gt;: > 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&gt; > 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&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. >>> >> >>> >> >>> > >>> >> >> >