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

Just to clarify a few more things. The use of CDIContainer container = CDIContainerLoader.getCDIContainer(); Was meant to emulate the boot process used in several other specs that have SE equivalents, e.g. JPA and Bean Validation, which use a locator type to give a factory, and that factory giving what you typically work with. John On Wed, Mar 4, 2015 at 7:59 AM Jozef Hartinger <jharting(a)redhat.com&gt; wrote: > 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. >>>> >> >>>> >> >>>> > >>>> >>> >>> >> > >