On Mar 25, 2007, at 7:40 PM, Remy Maucherat wrote:
David Jencks wrote:
I personally think the AnnotationProcessor is a very questionable
idea and hope no one uses it. However, it is pretty common now.
The point of the adapter is to show that tomcat can still support
people who want to integrate via something like
AnnotationProcessor. I certainly don't think tomcat should be
using a AnnotationProcessor wrapped in a LifecycleProvider. I was
basically trying to show that tomcat could work through an
interface like LifecycleProvider, and that it would provide
uniformity that is currently lacking, not trying to show a great
new implementation of the interface.
-- a little history and context --
I work on geronimo, and I started by getting annotation/injection
support to work on our app client container and jetty integration,
using some ideas I cribbed from OpenEjb and Xbean. With all of
these projects it was a really natural fit to have an object
creation service that, when given a class name, handed you back a
fully baked object. The code tended to get simpler and more
straightforward. Then I looked at MyFaces and realized that they
could also simplify their code by using an object creation service
where you say
getLifecycleProvider().newInstance(className);
rather than continual repetition of
Object o = clazz.newInstance();
getAnnotationProcessor().inject(o);
getAnnotationProcessor().postConstruct(o);
At this point, I don't like the patch much either. I am not
convinced by the proposed API (and the idea of an API change in
6.0.x) which is a bit more intrusive (a LifecycleProvider now would
have to be given to Jasper for it to work), and how it is better
than the "continual" (but explicit; quotes because it doesn't
exactly happen that often) repetition where it does matter (as far
as I can tell, not all the objects that may be instantiated may be
annotated).
AFAICT the objects that can be annotated are servlets, filters,
listeners, and tags. The objects that AFAICT can't be annotated at
present (unless jasper annotates some of its base classes, such as
with postConstruct methods) are compiled jsps and some kind of object
that jasper compiles out of tld files. I'm pretty sure that someone
who had more than my 2 days acquaintance with jasper could in a
couple of minutes point out how to avoid using LifecycleProvider or
AnnotationProcessor on generated classes. I might be mistaken but
I'm pretty sure the current code happily does call the
AnnotationProcessor on such classes. It's easy enough for jasper to
create its own LifecycleProvider if none is supplied, although my
current patch has null checks and alternate code for this circumstance.
At least one RI in JEE land (JSF) has independently adopted the
same AnnotationProcessor interface that is in use in Tomcat. I see
you like the LifecycleProvider API better, but are there really
things you cannot do (or are hard to do) with the
AnnotationProcessor API ? That would be the bar that would have to
be cleared IMO to justify a change.
Umm, could you explain how the jsf RI is "independent"? Of what? To
me that would mean they had come up with an interface with the same
three methods without having seen any examples of it. If you want to
count projects on either side of the fence, I got the idea from
OpenEjb, geronimo uses this idea, jetty is compatiible with it, and
MyFaces has adopted it after starting with the AnnotationProcessor
style interface.
The AnnotationProcessor style can't support constructor dependency
injection or factory methods. These are not envisioned by the specs
but there's nothing preventing their support through additional
metadata. An object creation service can. However, the main benefit
I can see for tomcat is that by swapping which implementation you use
at startup, you can specify the policy for object instantiation (such
as security sensitve, annotation sensitive, neither, custom.....)
without any runtime cost.
They agreed.
Then I looked at Tomcat. I found that there was a lot of
attention paid to all sorts of things such as security settings
and whether the class was in tomcat for servlets, but no such
attention paid for filters or listeners. I wondered if this was
an oversight.... I guess I should
Tomcat does not ship security sensitive filters and listeners, but
has some funny servlets. For sure I will not want to be performing
all these expensive checks for all instantiations, such as tags.
That certainly makes sense, but I don't see how it relates to the
current code in tomcat. From my reading the code that decides
whether to perform the extra security stuff
(SecurityUtil.isPackageProtectionEnabled()) doesn't depend on whether
the object being instantiated is from the tomcat classloader.
Therefore I thought it was reasonable to apply the same checks to all
managed classes, not just servlets.
There's also a check for explicitly restricted classes, which again
seemed to me reasonable to apply to all the kinds of stuff.
And finally there's this code the purpose of which is not all that
clear to me:
protected Class loadClass(String className, ClassLoader
classLoader) throws ClassNotFoundException {
if (className.startsWith("org.apache.catalina")) {
return containerClassLoader.loadClass(className);
}
try {
Class clazz = containerClassLoader.loadClass(className);
if (ContainerServlet.class.isAssignableFrom(clazz)) {
return clazz;
}
} catch (Throwable t) {
//ignore
}
return classLoader.loadClass(className);
}
What would happen if it was:
protected Class loadClass(String className, ClassLoader
classLoader) throws ClassNotFoundException {
try {
return classLoader.loadClass(className);
} catch (Throwable t) {
//ignore
}
return containerClassLoader.loadClass(className);
}
?
I was thinking that perhaps with something like the LifecycleProvider
interface tomcat could look at its configuration at startup and
decide which policy for object creation it wanted to use:
- security sensitive
- provided by tomcat- sensitive
- annotation aware
- ... you name it
and instantiate an appropriate factory that implemented that policy.
Then it could enforce the policy without any runtime checks to decide
which policy was in force.
One more time...
To me the benefits to tomcat of this approach are:
1. replace runtime checks with deploy time strategy installation
2. put all the object creation code in one place so its easier to see
if it makes sense
3. provide friendlier interfaces for projects wishing to embed tomcat.
To me (1) is self evidently a good idea and would be enough for me to
adopt the proposal. With regard to (2) I am not all that familiar
with tomcat, and having collected the existing object management code
into the LifecycleProviderToAnnotationProcessorAdapter class in my
patch, it doesn't make much sense to me. I was hoping that someone
more familiar with what is intended could either simplify it or
perhaps comment on the intent. I assume there are some circumstances
in which the more expensive checks involved in classloading are
appropriate, and I'm sure there are plenty in which they are not, so
I would expect that a deploy time strategy choice would make more
sense than continual runtime checks. As for (3).... it would have
been a lot less work for me to simply use the existing tomcat
interfaces, but this proposal looked to me like an obvious win-win
choice for both tomcat and geronimo.
thanks
david jencks
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
