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

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. >> >> >> >> >> > >> >