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: Thu, 26 Feb 2015 13:06:34 -0600

> On Feb 25, 2015, at 5:45 PM, Bill Shannon <bill.shannon_at_oracle.com> wrote:
>
> Jason Greene wrote on 02/25/15 14:08:
>>
>>> On Feb 24, 2015, at 6:07 PM, Bill Shannon <bill.shannon_at_oracle.com> wrote:
>>>
>>> In thinking about this a bit more, you bring up some good points.
>>>
>>> Part of what started us on this path of clarifying the expected behavior is
>>> that we discovered some bugs in this area in the RI, or at least, behavior
>>> that didn't match what we expected. That took us down some twisty paths
>>> that may not have ended up in the right place.
>>>
>>> Considering some of our history, it may be that some of this behavior is
>>> something we can't change. For example, in Java EE 5 we didn't have JNDI
>>> namespaces, but we did allow EJBs to be created by subclassing other
>>> classes that might have resource injection annotations. That seems to mean
>>> that the resource references have to be defined in the namespace of every
>>> component that subclasses the base class.
>>
>> Thereís one additional case I forgot to mention, where you can define new
>> components in XML, and those can reference/share the same backing classes.
>> Granted this is less common these days, but it runs into the same issue, and
>> can affect your algorithm below.
>
> You mean in deployment descriptors? Yes, I ignored that case below,
> but ultimately it needs to be incorporated into the algorithm.

Yes. I was just speaking to deployment descriptors.

>
>>> What we were trying to define is a simple processing model for annotations
>>> that would allow all processing to be done in a single pass over the
>>> classes.
>>>
>>> We expected every one of these resource annotations, on any class in the
>>> application, to define a resource reference entry in some JNDI namespace.
>>> The namespaces available to a given annotation on a given class were a
>>> function of where in the application the class was packaged. The
>>> deployment process that discovers these annotations could add the required
>>> information to the appropriate JNDI namespace without any information about
>>> how the class was used. And it could reject applications that attempted
>>> to declare resource references in namespaces that the class didn't have
>>> access to.
>>>
>>> What we're discovering is that this is too simplistic. We at least need to
>>> handle the case where a component class subclasses some other class. In
>>> this case, the base class may not have a component namespace of its own,
>>> but it needs to be able to define resources in the namespace of the
>>> component that subclasses it. Unfortunately, this means you need to know
>>> what all the "classes supporting injection" are, and you need to know the
>>> full class hierarchy for each of them, so that you can ensure the resource
>>> references from the superclasses appear in the component namespaces of all
>>> the component subclasses. Since "classes supporting injection" includes
>>> CDI managed beans, I don't know how you can know that at deployment time.
>>
>> Weld is a bit special in that it performs its own EE Resource injection,
>> using slightly different rules.
>
> Different than what? Is the result the same?

With CDI, @Resource (and the other EE injection bindings it supports) do not create JNDI bindings. They just act as a lookup mechanism to bridge the EE resource model with the CDI model.

>
> Injection of Java EE resources should be the same whether it's being done
> by Weld or something else.

So we could make it consistent, but we would need to define the namespace rules for it. Otherwise, it would lead to failures in CDI since they can occur on any class and those classes donít have component namespaces (the CDI spec and tutorials and so on do not set the name attribute on @Resource).

>
>> However in our case we also determine whether
>> or not a deployment contains a class that has ďbean defining annotationsĒ on
>> it, to determine whether or not Weld needs to be involved (also whether or
>> not a beans.xml has disabled weld processing).
>>
>>>
>>> It seems that we have to handle at least some cases of superclasses
>>> defining resources in the same namespace as the subclass. Sadly, you may
>>> be right that we have to handle that even in the case where the superclass
>>> is in a library
>>>
>>> Let's look at the general algorithm for initializing the JNDI namespaces.
>>> (We'll ignore deployment descriptor overriding for now. We'll also ignore
>>> application clients for now.)
>>
>> Iíll answer your question below first, by describing, very coarsely, how our
>> processing works. Our processing is component centric. We have a first pass
>> class structure and annotation scan which reads all the bytecode in a
>> deployment, including all possible places in the deployment classes can
>> exist, and constructs a set of shared indexes. We also do something similar
>> wit XML metadata, although each EE component layer has to register the
>> relevant parsing logic. From that information we have a number of fine
>> grained deployment processors (you can relate it to something like an
>> interceptor), installed by the various EE layers, which all collaborate to
>> create a set of component definitions. Those component definitions are
>> assembled with all of the JNDI binding information thats relevant to the
>> component.
>>
>> So in other words we notice every place that @Resource occurs, we just donít
>> do anything with that data until we process the component, and processing the
>> component involves analyzing the resource data for all classes that make up
>> the component.
>
> And how does that work for CDI managed beans? That's the case that motivated
> us to process annotations on all classes, not just some limited set of well
> defined easily discovered component classes.

We pass the annotation indexes to CDI, register a callback which is triggered on bootstrap, and that callback does a JNDI lookup. I donít think its possible to analyze without bootstrapping CDI since portable extensions can dynamically create annotations.


>
>>> 1. Find all the classes in each war file.
>>>
>>> 2. Find all the superclasses of each class from table EE-5.1 in each war
>>> file.
>>>
>>> 3. For all these classes, add any resource references or resource
>>> definitions in the java:comp namespace to the corresponding war component's
>>> JNDI namespace.
>>>
>>> 4. For all these classes, add any resource references or resource
>>> definitions in the java:module namespace to the corresponding war
>>> component's JNDI namespace.
>>>
>>> 5. Find all the classes in each EJB jar file that define an EJB.
>>>
>>> 6. Find all the superclasses of each of these classes.
>>>
>>> 7. For all these classes, add any resource references or resource
>>> definitions in the java:comp namespace to the corresponding EJB component's
>>> JNDI namespace.
>>>
>>> 8. For all these classes, add any resource references or resource
>>> definitions in the java:module namespace to the corresponding EJB jar's
>>> module JNDI namespace.
>>>
>>> 9. Find all the classes in each EJB jar file.
>>>
>>> 10. For all these classes, add any resource references or resource
>>> definitions in the java:module namespace to the corresponding EJB jar's
>>> module JNDI namespace.
>>>
>>> 11. For all classes in the application package, add any resource references
>>> or resource definitions in the java:app namespace to the application's JNDI
>>> namespace.
>>>
>>> 12. For all classes in the application package, add any resource references
>>> or resource definitions in the java:global namespace to the global JNDI
>>> namespace.
>>>
>>>
>>> A difficulty with the algorithm above is that table EE-5.1 includes CDI
>>> managed beans. Without initializing the CDI runtime, it's impossible to
>>> know what all the CDI managed beans are. In steps 2 and 9, we could just
>>> consider every class to be a potential CDI managed bean, adding or changing
>>> these steps:
>>
>> True, although CDI has a callback for this, since it discovers it for you.
>
> Right, but you need the information about all the resources the application
> requires *at deployment time*, before CDI is initialized, so that you can
> ensure all the resources are mapped to resources in the operational environment.
>
> The whole model of resource references in Java EE was that you could ensure
> at deployment time that everything the application needed was available, so
> that you didn't have failures at runtime. It's a very static model of
> resources, which clearly has advantages and disadvantages, but this has
> always been the model.

So even though CDI made things a little more dynamic, all of the dynamic bits happen eagerly, so the callback for every EE injection point happens during bootstrap before the application code is actually running, so it should be possible to validate and/or alter the lookup values if needed.

For the CDI RI there are some impl details here (see InjectionServices):
https://github.com/weld/core/blob/master/docs/reference/src/main/asciidoc/ri-spi.asciidoc

>
>>> 2. Find every superclass of these classes.
>>>
>>> 9a. Find every superclass of these classes.
>>>
>>>
>>> Oh, but we forgot about interceptors, which have their own special rules.
>>> Add these steps:
>>>
>>> 2a. Find all the interceptors for each of these classes.
>>>
>>> 2b. Find all the superclasses of all these interceptor classes.
>>>
>>> 5a. Find all the interceptors for each of these classes.
>>>
>>> 5b. Find all the superclasses of all these interceptor classes.
>>>
>>> It's not clear from the Interceptors spec whether these special rules for
>>> interceptors apply to only those interceptors declared using @Interceptors,
>>> or also to those declared using interceptor bindings. Again, the latter are
>>> essentially impossible to determine without initializing CDI, so we'll
>>> assume only the former was intended for now. Since the genesis of this
>>> rule was from the EJB spec, we could further limit this to @Interceptors
>>> applied to EJBs if desired, which would remove steps 2a and 2b.
>>>
>>>
>>> What's missing from the algorithm is any attempt to detect the error where
>>> a class declares a resource in a namespace that's unavailable to the class.
>>> To handle that, we add these steps:
>>>
>>> 13. If any class in an ejb jar file and not in the set from step 6 declares
>>> a resource in the java:comp namespace, it's an error.
>>>
>>> 14. If any class in a library and not in the sets from steps 2 and 6
>>> declares a resource in the java:comp or java:module namespace, it's an
>>> error.
>>>
>>> 15. If any class in a rar module and not in the sets from steps 2 and 6
>>> declares a resource in any java: namespace, it's an error.
>>>
>>> Ideally these would be deployment errors since it's better to detect
>>> application errors early, but clearly that comes at some cost.
>>>
>>> Does this algorithm better match what you're expecting, and hopefully
>>> implementing?
>>
>> This certainly sounds better. Although, if I understand correctly, there is
>> still some confusing behavior. For example, unqualified names
>> (@Resource(name=ďblahĒ)) will fail on a non-component class, unless extended
>> by a component class (or defined in XML). If that component class is retired
>> then we now have a failure.
>
> What do you mean by "retiredĒ?

Deleted or undeclared in a DD.

>
> Yes, if you include classes in your application that aren't used, and those
> classes reference resources that don't exist, or define resources in a
> namespace unavailable to the class, deployment should fail.
>
> Our goal has always been to detect these errors as soon as possible, usually
> at deployment time. It's impossible to know whether these errors will cause
> the application to fail or will be harmless.

I guess it depends on how you view it. My mental model has always been that EE annotation metadata is just a bunch of configuration snippets (each of which are neither valid nor invalid) that ultimately gets assembled and merged into the real and final configuration, and thatís the piece that gets validated.

>
>> I know we are supposed to be discussing the ideal behavior and not get into
>> whether or not its an MR. However, I think its important to note thereís
>> still that case where JSR-250 annotations are used by some third party
>> framework, since they are generic, and now the EE container wants to take
>> control of those. Thatís probably ok for EE8. Although, at least, in our case
>> we have already shipped an EE7 server (WildFly 8.0-8.2), so breaking them in
>> a patch release (to conform to an MR) seems a bit unfair to them. Granted we
>> could provide a flag for them to get them by if it was decided to go this
>> route.
>
> We never intended that JSR-250 annotations could be used in a Java EE
> application for something other than what Java EE defines.

I wasnít a participant back then, so you know better than me. All I can say is that I donít think framework developers got that message though. Since the spec is called ďcommon annotationsĒ, and it was included in Java SE, it sends a completely different message. To use an example, Spring overloads these, and making this sort of change in an MR would cause Spring deployments to start failing when they worked just fine on all previous versions.

To be clear though, Iím not arguing we should let third-party frameworks dictate the EE spec (that would be a bit silly). My concern is breaking stuff in a maintenance release, and that causes users using these frameworks to seek other solutions.

>
> That's different than JSR-330 annotations, which we knew were being used
> outside of CDI, and we wanted to allow products to support applications
> with those annotations but without CDI interpreting those annotations.
>
> If a Java EE product isn't always processing JSR-250 annotations as required
> by the Java EE spec, it's not compatible with the spec.
>
>> One thing I wonder though, is does this really bring enough value to justify
>> the cost of the change? For example, it takes just one extra annotation to
>> make a class a component (e.g. ManagedBean/Singleton/ApplicationScoped etc).
>
> I'm not sure what change you're referring to.
>
> We haven't been trying to *change* anything, only to clarify what we
> originally intended. Admittedly our first attempt wasn't quite right,
> but I'm hoping we're converging on agreement of the original intent.

We previously had a restricted set of classes which had injection performed on them (EE components), and AFAICT, both algorithms expand it to cover all classes. This is effectively introducing a new kind of quasi EE-component.


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