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

2015-03-04 12:29 GMT+01:00 John D. Ament <john.d.ament(a)gmail.com&gt;: > Hmm.. comments in line. > > On Wed, Mar 4, 2015 at 4:49 AM Antoine Sabot-Durand < > antoine(a)sabot-durand.net&gt; wrote: > >> 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 >> > > It seems odd to me that we're rehashing decisions made during the EG > meetings. Not putting it in CDI was discussed in several meetings at the > beginning of the year, and it seemed like the agreement was putting it in > CDI was a bad idea. > > >> - 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 >> > > Agreed, but I think we need to provide some entry point to allow those > who aren't comfortable with programming with events to leverage it. > Returning the CDI instance makes that easier to do than returning the > BeanManager. > > Can you detail it please? CDI value is only its getBeanManager() method IMO so it sounds like it is 1-1 for me.

> >> 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. >> >> > We talked about this one as well on the EG, either this year or late last > year. I thought the decision at that time was that we wouldn't allow > multiple containers at once in SE. > > wouldn't specify rather than wouldn't allow I think > Antoine >> >> >> Le 1 mars 2015 à 15:13, John D. Ament <john.d.ament(a)gmail.com&gt; 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&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/2c362161e18dd521f8e8 >>>>> 3c27151ddad467a1c01c >>>>> >>>>>> >>>>> >>>>>> 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 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. >