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

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/ 2c362161e18dd521f8e83c27151ddad467a1c01c >>>>>> >>>>>> 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. >> >> >