users@javaee-spec.java.net

[javaee-spec users] [jsr366-experts] Re: Compatibility Problems with MR Resource Annotation Widening

From: Jason Greene <jason.greene_at_redhat.com>
Date: Mon, 2 Mar 2015 18:07:51 -0600

> On Mar 2, 2015, at 5:09 PM, Jason Greene <jason.greene_at_redhat.com> wrote:
>
>
>> On Feb 27, 2015, at 5:38 PM, Bill Shannon <bill.shannon_at_oracle.com> wrote:
>>
>> You learn something every day...
>>
>> Before CDI, all deployment processing could be done without running,
>> or even loading, any classes in the application.
>>
>> Apparently I wasn't watching carefully when CDI was being defined,
>> and in particular when the Weld-specific integration SPI was defined.
>> It now appears that CDI has to be bootstrapped before you can
>> determine whether deployment succeeds or fails. This wouldn't be
>> so bad if CDI was just another "container" that needed to look at
>> application metadata when the application was deployed, but the
>> introduction of CDI portable extensions means that code in the
>> application has to run during the application deployment process.
>> I'm not sure we've fully defined the environment in which that code
>> is running - what resources are available, what APIs it's allowed
>> to use, what happens if it does something it shouldn't do, whether
>> it's allowed to access the database or create threads, etc. I also
>> don't understand what's supposed to happen if I deploy my application
>> without starting it.
>
> The only code thats allowed to execute from a CDI perspective, is the portable extension class, which uses the service loader mechanism. Only when deployment has completed can code in the CDI component classes execute.
>
>
>> Does CDI bootstrapping occur so that deployment
>> can succeed, and then the application is terminated? Maybe I need to
>> spend more time with that part of the CDI spec…
>
> The application doesn’t have to start at all, with the Weld SPI in particular there is an endInitilization SPI call that you can just never call. You could just shutdown the CDI container right at that point. Or, you could wait until some point at which the server is ready.
>
>>
>> Jason Greene wrote on 02/26/15 19:02:
>>>> These annotations need to be processed at deployment time in order to work
>>>> properly. If you wait until CDI is bootstrapped, it's too late. So you
>>>> need to process these annotations on all classes, whether CID is using them
>>>> or not, so that they'll work properly.
>>>
>>> Why? I don’t see how this is required. The CDI RI bootstrap is broken into
>>> separate phases, and that allows you to analyze all this stuff, and do it
>>> cooperatively/in-parallel before user code is ever executed, and certainly at
>>> deployment time. Now maybe the EE RI needs something else out of the CDI RI
>>> SPI, but thats really just an implementation issue.
>>>
>>> The one exception is portable extensions, as those are a container SPI
>>> contract, and participate during deployment and undeployment.
>>>
>>>>
>>>> These classes need to be able to contain resource *definitions*, not just
>>>> references. And as I said above, they need to define resource references
>>>> that can be overridden in the deployment descriptor.
>>>>
>>>> (As far as I know, there's no way for a CDI extension to dynamically add
>>>> (e.g.) an @Resource annotation to a class. And again, since the
>>>> information from those annotations needs to be visible at deployment time,
>>>> that's not going to work.)
>>>
>>> A portable extension can emit any injection point, including a resource
>>> injection point and the generic callback should still work since the
>>> container wouldn’t know the difference (unless it looked at the bytecode
>>> itself).
>>>
>>> This looks like it should work on the EE RI (I haven’t tried it though):
>>>
>>> https://java.net/projects/glassfish/sources/svn/content/trunk/main/appserver/web/weld-integration/src/main/java/org/glassfish/weld/services/InjectionServicesImpl.java?rev=63779
>>
>> Ok, so it may actually be *possible* for CDI to collect all the information
>> about resource annotations on CDI managed beans at deployment time,
>> communicate that to the container, and allow the container to process
>> those annotations and configure those resources.
>>
>>>> Before CDI, the set of classes was quite restricted. These were the
>>>> "container managed" classes. These were the classes instantiated by the
>>>> container, not by the application, which gave the container the
>>>> opportunity to perform injection. Clearly many people would like injection
>>>> to occur for "new", but that's not going to happen, so we described which
>>>> classes the container was managing, and thus which classes we could
>>>> actually make injection work on.
>>>>
>>>> After CDI, we expanded the set to include all the container managed
>>>> classes that CDI managed, which meant almost anything that could be used as
>>>> a CDI bean. The only way to make injection work in all those cases is to
>>>> consider every class a potential container managed class and to process
>>>> these resource reference and definition annotations on all these classes.
>>>> That's why Chapter 8 says you have to collect this deployment information
>>>> (and again, resource reference and definition annotations are just another
>>>> way of specifying deployment information) from all classes at deployment
>>>> time.
>>>>
>>>> I don't know how else to make the requirements of the spec actually work
>>>> correctly, which is why we viewed this as a clarification, not a change.
>>>
>>> If its just a matter of saying all of these types have to be looked at, then
>>> I think we have already defined that, since all of the relevant specs specify
>>> which types are eligible. If, however, theres a new set of classes that are
>>> receiving injection & bindings, then that’s by definition a scope expansion.
>>
>> No new classes are receiving injection.
>>
>> The inconsistency we're trying to clarify is between Chapter 5, which says
>> that resource definition annotations can only appear on container-managed
>> classes, and Chapter 8, which says that all classes must be scanned for
>> annotations that specify deployment information.
>>
>> As I explained, there's some spec requirements, and use cases, that can't
>> be implemented properly unless all classes are scanned. That's why it
>> seems to us that this is just a clarification, not a change, and not an
>> expansion in scope.
>
> I guess I just don’t agree that having to scan an annotation means you have to define a component. To me this seems to be a bit of a false dichotomy. We can certainly improve things without making every Java class a component.
>
>>
>> If, in fact, the CDI implementation is discovering all the CDI managed beans
>> at deployment time, and communicating that information to the container so
>> that it can discover all the resource reference and definition annotations
>> on those classes, we may be closer than I thought. Is CDI/Weld doing that?
>
> Yes but only for valid injection points, so if its not a Resource producer then it wouldn’t be in that particular callback. That said the EE container can participate in a number of ways as part of the process. And the SPI could also be extended further.
>
>>
>> If so, then we know that all the other classes don't support injection, and
>> thus shouldn't contain any annotations specifying injection. If they did,
>> and those annotations were ignored, that might be a gap we could live with.
>>
>> That leaves us with these other classes specifying resource references or
>> definitions at the class level. Since these classes can use JNDI to look up
>> these resources at runtime, it seems perfectly reasonable for them to be able
>> to specify resource references or definitions. You have to scan all the
>> classes to find all the class level annotations anyway, so discovering
>> these class level annotations should be essentially no additional cost.
>> It seems like this is a perfectly reasonable use case that should work,
>> and as we saw from my earlier poll, many developers expect this to work.
>> And it works in the RI.
>
> Glassfish does not appear to be defining bindings on the CDI @Resource values, unless I am looking at the wrong code, or its duplicated in some other place (sorry if I am wrong about this, I still haven’t had time to test it) .

Actually I misread, you were clearly talking about class annotations. In that case the MR description behavior you are advocating did not match the behavior I observed there either (I mentioned that in a previous email). It can’t work unless you also fix the way namespaces are handled. Even doing that its going to break compatibility, for largely no reason.

--
Jason T. Greene
WildFly Lead / JBoss EAP Platform Architect
JBoss, a division of Red Hat