ProcessInjectionPoint clarification for Instance injection points

[manovotn]

I think the documentation around ProcessInjectionPoint and its getInjectionPoint() would deserve a clarification.
And possibly for InjectionPoint#getType() as well.
Currently, it is pretty unclear what will happen if your bean has an injection point with Instance<X> and you are trying to observe that.

Given following bean with its injection point:

@Dependent
public class MyBean {

    @Inject
    Instance<Foo> instanceFoo;

}

And the following extension with PIP observer:

public class MyExtension implements Extension {

    public void observe(@Observes ProcessInjectionPoint<MyBean, X> pip) {
        // 1) is X going to be `Foo` or `Instance<Foo>`?
       pip.getInjectionPoint().getType();
    }

    public void observe(@Observes ProcessInjectionPoint<MyBean, Instance<Foo>> pip) {
        // 2) observing for Instance<Foo>, should this observer trigger?
       pip.getInjectionPoint().getType();
    }

    public void observe(@Observes ProcessInjectionPoint<MyBean, Foo> pip) {
        // 3) observing for IP of type Foo directly, should this observer trigger?
       pip.getInjectionPoint().getType();
    }

}

If you go looking into docs and javadoc, the notable parts are:

• ProcessInjectionPoint#getInjectionPoint()

  • getInjectionPoint() returns the InjectionPoint object that will be used by the container to perform injection.

• InjectionPoint#getType():

  • The getType() and getQualifiers() methods return the required type and required qualifiers of the injection point. If the injection point represents a dynamically obtained instance, the getType() and getQualifiers() methods should return the required type (as defined by Instance.select()), and required qualifiers of the injection point including any additional required qualifiers (as defined by Instance.select()).

• At the same time, this is a definition from built-in instance bean stating that required type is basically Instance<X>

  • The container must provide a built-in bean that:

  • is eligible for injection to any injection point with required type Instance or Provider, for any legal bean type X,

Few side notes:

• It is IMO more practical to be able to observe ProcessInjectionPoint<T, Instance<Foo>> as otherwise you are unable to differentiate between classic and Instance-based injection points
• You can also configure injection points so you should be able to weave in or tweak Instance injection points - again, for that reason you should IMO be able to observe that directly
• Lastly, if you are curious what Weld does, here is a draft of tests in Weld that show the behavior there. In short, in the above scenario, Weld would treat the injection point as Instance<Foo>.

Thoughts?

[Ladicek]

Without too much thinking about this, it seems to me that observing Instance<Foo> is more useful and is likely expected by most extension authors.

[Azquelt]

If the injection point represents a dynamically obtained instance, the getType() and getQualifiers() methods should return the required type (as defined by Instance.select()), and required qualifiers of the injection point including any additional required qualifiers (as defined by Instance.select()).

This seems like the only part that would possibly contradict your conclusion, and it reads to me like it relates to the expected values if you inject an InjectionPoint into a @Dependent scoped bean? If so, maybe this part should be clarified to say that it only applies when the injection point metadata is injected into a bean? (possibly with more precise language than that)

The consequence of that would be that if you do

@Inject Instance<Foo> fooInstance;

then in an extension ProcessInjectionPoint.getInjectionPoint().getType() would return Instance<Foo>.

However, if you had

@Dependent
public class Foo {
    @Inject InjectionPoint ip;
    ....
}

then for a Foo obtained via fooInstance.get(), ip.getInjectionPoint().getType() would return Foo.

[Azquelt]

Actually reading the spec for Injection point metadata again, I think it's fairly clear that it's expected that you get a different InjectionPoint for the Instance itself and for the bean obtained from the instance:

An instance of InjectionPoint may represent:

• an injected field or a parameter of a bean constructor, initializer method, producer method, disposer method or observer method, or
• an instance obtained dynamically using Instance.get().

The Instance<Foo> field seen by the extension would be the first case, whereas Foo.ip would be the second case.

[manovotn]

So you think no clarification is needed?

I am not yet sure what I'd propose as exact wording but I feel like we should at least try to specify that the text under InjectionPoint#getType() references only the case where you inject InjectionPoint into a dependent bean.

[Azquelt]

From a technical point of view I don't think a clarification is needed, but if we can find a way of making it easier to read and understand, I think we should.

I did try to think about how I would change it, and I ran into a problem of ordering.

At the moment we first talk about what InjectionPoint can represent, then what its methods do, then how it can be injected into a @Dependent scoped bean. The part about using it in an extension isn't mentioned until we get to the Portable Extensions chapter.

I guess we could either:

• When talking about what it represents and its methods, refer forwards to its use being injected or in an extension
• When talking about how it can be injected and used in an extension, explain more explicitly what the InjectionPoint represents (i.e. in an extesion, it always represents a field, method parameter etc., whereas when injected it depends on whether the bean was looked up dynamically from an instance or not)

[manovotn]

When talking about how it can be injected and used in an extension, explain more explicitly what the InjectionPoint represents (i.e. in an extesion, it always represents a field, method parameter etc., whereas when injected it depends on whether the bean was looked up dynamically from an instance or not)

+1, this makes more sense to me although both would work.