[cdi-dev] Feedback - CDI bootstrap API (CDI-26)
Jozef Hartinger
jharting at redhat.com
Fri Feb 27 02:05:04 EST 2015
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
> <mailto: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
> <mailto: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 <mailto: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 <mailto: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
> <mailto: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 <mailto: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/20150227/cc9f6380/attachment-0001.html
More information about the cdi-dev
mailing list