Closing all resolved issues in CDI 1.x

Craig, the issue for EntityListener injection is CDI-144

I created CDI-155 for the Instance<T>#destroy() issue to prevent mem leaks with @Dependent beans.

Regarding JPA Entity Listeners, that's going to be fixed in JPA 2.1 ; I got sick of it the issue and nagged the spec team about it. See sections 3.5-3.5.2 in the latest draft (http://java.net/projects/jpa-spec/downloads). Please review it now, don't wait to complain about it once it goes final!

Initial proposal, which deviates totally from what we discussed here at https://github.com/jboss/cdi/pull/43

Issue with all of this is that now destroying this instance is hard. We would need to extend Instance<> with a void destroy(T instance); method.

We cannot split the BeanManager more than in 1.0, but we can add new methods (like the method discussed here) somewhere else.

Mark, you can fire an event when you have an Instance<Object>:

Instance<Object> instance = ...
Event<MyEvent> event = instance.select(new TypeLiteral<Event<MyEvent>>() {}.getType()).get();
event.fire(...);

Since event is a bean, this should work.

Yes as previously commented I was saying simply that if we made it available outside of a managed object it would be via the same mechanisms that we use for all other artifacts from CDI you can look up like this (i.e. currently BM). I'm not in love with the idea that we expose Instance directly, as I think requiring extension authors to write:

Foo foo = beanManager.dynamic().select(Foo.class).get();

(or whatever else this method get's called) is not that onerous given this is extension authors only.

With hindsight I would have preferred to split up BeanManager more than was done in 1.0 but we can't got back and do that now!

But then you hit the scenario that you like to fire an event from a JPA EntityListener (had this recently) and you need the whole BeanManager again

But I'm completely d'accord with you that using the BeanManager or any other SPI outside an Extension should only be a really exceptional case!

It must not be JNDI. We can get the BeanManager from elsewhere (a ThreadLocal or a static singleton from somewhere).

My point was the same as Petes: The BeanManager is for extension developers and they can get it injected, so no need to access it from elsewhere.
Application developers (or users of CDI like Pete called them) should not need to obtain a BeanManager. The only thing they need is to get a contextual reference from within an unmanaged object (like a JPA EntityListener or a Struts Action or other unusual cases). And the ObjectInstance would be great for this. And additionally it provides the "give me all beans of a certain type" access.

Another option would be to provide a SPI to inject into unmanaged instances. Something like BeanManager#inject(Object). But again this needs not to be the BeanManager, because it will be used by application developers and the BeanManager is used by Extension developers.

Mark, that´s why the java:comp/env namespace exists from. In fact, each app server has it´s own descriptor in order to map java:comp/env references to external (appserver) bindings. But that´s another discussion.

Maybe Pete´s previous statement could lead to a right direction. Having a facade API for BeanManager could do the job.

I'd keep it in the BeanManager. Having to code 1 static singleton lookup is ugly enough.

Btw, please NOT JNDI again. JNDI really sucks! The JNDI spec is a splitter bomb cross over the whole JCP. You will find 20++ JSRs which define name spaces + behaviour, etc. Parts got cleaned up in EE6 (or was it 5?), but it's still broken and non-portable across AppServers to big degrees.

An example: try to register a Seam2/Hibernate datasource which works in different EE Servers without changing the JNDI name... This is just broken...

Also: there are a lot environments which either not have a JNDI environment at all, or it's readonly.

Just my 0.02 ...

To be clear BeanManager is an SPI for integration purposes, it's neither an internal API or a user facing API

RE: a) you were looking in the wrong place

Wouldn't be the first time.

RE: b) the spec did a very very bad job of showing you the right place to look (actually it showed you the wrong place!)

I think the issue is that the BeanManager is the "thing" you look up in JNDI. The thing you look up in JNDI is usually the interface that end users deal with not an internal API. That said I looked at it with a "Spring perspective" because that is what I have been dealing with for the past 9 years or so.

Interesting idea Arne. We already have some discussion about how to do this for BM, perhaps we can do the same for Instance. We could certainly register it in JNDI.

Note that the instances obtained in this way are non-contextual (or unmanaged) instances.

We even could go a step further and provide an interface to the application developer without the need of a bean-manager.
What about adding the following class to the spec:

public class ObjectInstance implements Instance<Object> {
    
    private Instance<Object> delegate;

    public ObjectInstance() {
        //lookup bean-manager via JNDI,
        //get Instance<Object> from bean-manager
        //and set delegate
    }

    //methods are delegated to the delegate
}

Every CDI-user could instantiate this class and use it to get contextual instances.

Rick, I think this points more to a communication issue than an API failing. BeanManager is intended as a low level API (in fact it is even labeled an SPI). Instance<> is intended as a user application facing API (so not low level at all). It's far more user friendly than BeanManager (it just focuses on the case of programmatic instantiation, none of the other stuff that is on BeanManager), and offers exactly the methods you want. In fact Lincoln's utility would

a) introduce a smaller learning curve
b) be more powerful

if rather than inventing a new API for this, simply allowed you to look grab an Instance<> (to be fair it took me well over a year to even realise this problem was trivial to address as well .

So in response to your "Spring app context" point, what I would say is that the issue isn't that BM doesn't have the right methods, it's just that

a) you were looking in the wrong place
b) the spec did a very very bad job of showing you the right place to look (actually it showed you the wrong place!)

Let's fix this.

Rick, open an issue for allowing Instance to do a name based selection and give your use cases there.

I can also say that there was a lot of cussing and gnashing of the teeth when I did not get a getBeanByType type method. I was upset.

Peter et al, good points. So Instance is the way to do this, and if you expose and instance, you can use instance. Got it.

Lincoln, We wrote a utility class as well and we use that a lot. I don't think we are normal users (in every sense of the word) .

I just think adding a couple of extra methods that hide the details of Instance is a good idea for an easier to use API.
I am guessing that not everyone wants to be an expert on CDI, but they may want to grab a bean.

I know I was quite surprised the amount of learning curve it took me to just look a bean up by name. I compare this to the Spring applicationContext, which does have these types of methods. I just think that BeanManager should have some easy to use facade methods to do common things.

There are a lot of people who don't care so much about instance and low level details of CDI. They just want a darn bean.

Returning an Instance<Object> method is the right thing to do here IMO. Adding a select(name) method is probably something we should consider even if not requested immediately.

The original proposal is to add a utility method to BeanManager which returns an Instance<Object> which can be used to do programmatic lookup. Apart from the isDependentScoped method Instance exposes all of the methods you have above, and also allows for subselection. We can add a select(String beanName) to Instance easily if needed (Rick indicates that it is). This prevents us duplicating an existing API and means we don't have to make BM much more complex.

@Inject private Instance<Object> obj;

only works when injection is already available; I believe they are asking for methods on the beanManager to reduce things like the BeanManagerUtils that I wrote, which even on the Seam Team, we use everywhere to get instances of contextual objects.

package org.jboss.seam.faces.util;

import java.util.ArrayList;
import java.util.List;

import javax.enterprise.context.Dependent;
import javax.enterprise.context.spi.CreationalContext;
import javax.enterprise.inject.spi.AnnotatedType;
import javax.enterprise.inject.spi.Bean;
import javax.enterprise.inject.spi.BeanManager;
import javax.enterprise.inject.spi.InjectionTarget;
import javax.inject.Inject;

/**
* A utility providing common functions to simply use of {@link BeanManager}
*
* @author <a href="mailto:lincolnbaxter@gmail.com>Lincoln Baxter, III</a>
*/
public class BeanManagerUtils {
    @Inject
    private BeanManager manager;

    /**
* Determine if a bean is {@link Dependent} scoped.
*/
    @SuppressWarnings("unchecked")
    public <T> boolean isDependentScoped(final Class<T> type) {
        Bean<T> bean = (Bean<T>) manager.resolve(manager.getBeans(type));
        if (bean != null) {
            return Dependent.class.equals(bean.getScope());
        }
        return false;
    }

    /**
* Get a single CDI managed instance of a specific class. Return only the first result if multiple beans are available.
*
* @param type The class for which to return an instance.
* @return The managed instance, or null if none could be provided.
*/
    public <T> T getContextualInstance(final Class<T> type) {
        return getContextualInstance(manager, type);
    }

    /**
* Get a single CDI managed instance of a specific class. Return only the first result if multiple beans are available.
* <p/>
* <b>NOTE:</b> Using this method should be avoided if container provided injection is available.
*
* @param manager The bean manager with which to perform the lookup.
* @param type The class for which to return an instance.
* @return The managed instance, or null if none could be provided.
*/
    @SuppressWarnings("unchecked")
    public static <T> T getContextualInstance(final BeanManager manager, final Class<T> type) {
        T result = null;
        Bean<T> bean = (Bean<T>) manager.resolve(manager.getBeans(type));
        if (bean != null) {
            CreationalContext<T> context = manager.createCreationalContext(bean);
            if (context != null) {
                result = (T) manager.getReference(bean, type, context);
            }
        }
        return result;
    }

    /**
* Get all CDI managed instances of a specific class. Return results in a {@link List} in no specific order.
*
* @param type The class for which to return instances.
*/
    @SuppressWarnings("unchecked")
    public <T> List<T> getContextualInstances(final Class<T> type) {
        List<T> result = new ArrayList<T>();
        for (Bean<?> bean : manager.getBeans(type)) {
            CreationalContext<T> context = (CreationalContext<T>) manager.createCreationalContext(bean);
            if (context != null) {
                result.add((T) manager.getReference(bean, type, context));
            }
        }
        return result;
    }
}

Rick, George, why do you want to create a new API for programmatic lookup when we already have Instance? What is your motivation for adding methods to BeanManager over exposing/enhancing the existing API?

Fab, yes we should realy make it clear that those new and all the other BeanManager stuff is only intended for hardcore use (e.g. in parts where you dont have injection available) and should normally not be needed.

regarding getReference(): this returns you a proxy ("Contextual Reference") for the internal contextual instances. So this stuff will give you lifecycle managed instances only.

The proposal on Richard is great, as it resembles Component class from Seam 2. This is a very useful feature that IMHO should not be left behind on this spec.

Some utility methods on the BeanManager for making common operations equate to oneliners would be nice....

public Object getBeanByName(String name);
public <T> T getBeanByName(Class<T> type, String name);
public <T> T getBeanByType(Class<T> type, Annotation... qualifiers);

Or perhaps just add a utility class that takes a BeanManager and provides these if we don't want to clutter up BeanManager.

This would be for cases when a developer can't use injection.

Note that I am not in love with the method name at all! If we can find a better name then that would be great

Given we already have a method called getReference with somewhat different semantics I don't think we should overload it's meaning. Furthermore, Instance<> is the API for "dynamic" injection, and we should not introduce a new API for this. My proposal simply makes this API more accessible in non-managed component.

agree

> So a BeanManager#getReference(T, Annotation...) or a BeanManager#instance() would be really desirable.

Imo getReference would be easier to type because getInstance().select(qualifiers).get() is more complex.

Otoh, there might be use cases where you like to get the power of Instance() without having to go through BeanManager#getBeans ?

This has been one of the biggest complaints I have heard about the CDI spec so far (actually Mark, you're the only person I've heard so far say we shouldn't make this a single call).

A couple of specific comments:

• BM is primarily intended as an SPI for providing access to CDI in unmanaged instances, so if we need to add better support for providing injection in unmanaged instances (see above) then BM is the right place to do it.
• instance() is indeed sufficient as Instance() allows fluent selection anyway.

Anyone else have an opinion on this?

In the CDI-project I currently work on, this three calls (BeanManager#getBeans, BeanManager#resolve and BeanManager#getReference) where the first thing a developer wrote a BeanManagerUtil#getReference(BeanManager, T, Annotation...) for. So I think, it are just three lines, but it are three lines that really should be doable in one line. So a BeanManager#getReference(T, Annotation...) or a BeanManager#instance() would be really desirable.

BTW: instance() is enough since one could call instance().select(...), which would be a one-liner, too.

To clarify this a bit:
Currently we need

• BeanManager#getBeans
• BeanManager#resolve
• BeanManager#getReference

I don't think
Instance<Object> instance();
is enough.

It should be more like
Instance<T> instance(T, Annotation<? extends Qualifier>);
right?

But to be honest: imo we should not make too much functionality available in the BeanManager and bloat our interface if it is easily doable already. This is really only used in situations where you cannot @Inject your Instance object (means in unmanaged instances like a JPA EntityListener or similar). But in those rare cases, it's easily doable already. Maybe we just add an own section to the spec and explain how this 3 methods work together for getting the contextual reference?