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