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

Romain Manni-Bucau rmannibucau at gmail.com
Wed Mar 4 07:47:54 EST 2015


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 at 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-dev.2015-01-14-17.04.log.html
>
>  John
>
> On Wed, Mar 4, 2015 at 4:05 AM Jozef Hartinger <jharting at 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 at 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 at 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 at 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 at 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 at 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 at 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/2c362161e18dd521f8e83c27151ddad467a1c01c
>>> >>>>>>
>>> >>>>>> Let me know your thoughts.
>>> >>>>>>
>>> >>>>>> Thanks,
>>> >>>>>>
>>> >>>>>> John
>>> >>>>>>
>>> >>>>>>
>>> >>>>>> _______________________________________________
>>> >>>>>> cdi-dev mailing list
>>> >>>>>> cdi-dev at 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 at 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.
>>> >>
>>> >>
>>> >
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/cdi-dev/attachments/20150304/0c40b17c/attachment-0001.html 


More information about the cdi-dev mailing list