looks like the patch might introduce some funky dependencies, it got
neglected
http://marc.info/?l=tomcat-dev&m=117477476413027&w=2
The interface lifecycle provider, you put in o.a.catalina, thus creating
a funky relationship.
remember, that other containers use jasper, now they'd have to embed
tomcat to do so.
The patch probably can be a lot easier, if you want to inject your own
annotation processor, just do so by
StandardContext.setAnnotationProcessor, and the default one will not be
created.
for example, right now we are looking at code something like this
Tag instance = (Tag) handlerClass.newInstance();
AnnotationHelper.postConstruct(annotationProcessor, instance);
the patch proposes to handle these two lines (plus the actual
classloading) in a single line, by using your method. The feedback was
that the patch introduced some code complexity that is not really needed.
Especially when you already can inject your own annotation processor
into the mix.
I'll try to draw out some more constructive criticism out of the group,
but I need to better understand the need for Geronimo here, based on
that there might be some other ideas that come up.
If there simply is a patch to tomcat, with no added benefit to tomcat,
then I'm afraid it will be hard to justify. And I don't think you wanna
fork parts of the tomcat tree, it will not be popular.
Filip
David Jencks wrote:
I've been working on connecting geronimo annotation processing and
injection support to tomcat and jasper and studying how the current
tomcat and jasper support this and have been working on a proposal for
changing how tomcat deals with this area that I hope will be seen as
an improvement.
I've generally removed exceptions from the method signatures below in
an attempt to make the situation a little clearer.
As far as I can tell (so far) there are four kinds of objects relevant
here:
servlets
filters
listeners
tags
The first three are created more or less directly by tomcat whereas
tags are created by some code in jasper and by code generated by jasper.
Currently tomcat and jasper use a very wide variety of techniques to
create the objects and then use an instance of
public interface AnnotationProcessor {
public void postConstruct(Object instance);
public void preDestroy(Object instance);
public void processAnnotations(Object instance);
}
in a sequence like this:
Object o = <create a new instance through various kinds of magic>
annotationProcessor.processAnnotations(o);
annotationProcessor.postConstruct(o);
When its time to toss the object they call
annotationProcessor.preDestroy(o);
What I would like to do is replace the AnnotationProcessor with a
different simpler interface like this: (the name is not important...)
public interface LifecycleProvider {
Object newInstance(String fqcn, ClassLoader classLoader);
void destroyInstance(Object o);
}
The idea is that the newInstance method does everything necessary to
construct a fully injected and post-constructed instance, and the
destroyInstance does everything necessary to stop it. Its very easy
to write an adapter between this proposed interface and the
AnnotationProcessor interface, so tomcat and jasper would continue to
support the AnnotationProcessor approach just as they do today.
The reason use this interface from geronimo's perspective is that we
have some very nice code that can do the object creation and injection
in one step. It's designed to support constructor injection as well
as property injection, so the object instance construction and
injection aren't really separable.
Aside from altruism the reason I think the tomcat developers might be
interested in this is that there is such a wide variety of code in the
<create a new instance through various kinds of magic> step and it
looks to me as if this is most likely a consequence of less and less
attention getting paid as new kinds of objects need to be created.
This would put all the managed object creation code in one place so
each object creation would get the same level of attention.
For instance, while listeners and tags are created with a simple
clazz.newInstance(), the servlet construction code checks a lot of
conditions before deciding how to construct the object: in particular
security settings might cause it to be in a PrivilegedAction and if it
is available in the same classloader as tomcat then special actions
are taken. While I don't entirely understand the point of some of
this it seems highly unlikely that it is appropriate only for servlets
and not filters, listeners and tags.
I've been working on this approach for about a week now and think I
have most everything related to tomcat changes working. There are
still some problems with things like tld schema upgrades which are not
related to tomcat code. I would be more comfortable proposing this to
the tomcat community after we've ironed out more of the geronimo
problems, but I'd like to start some discussion on this and also
commit my code so everyone can see at least the geronimo side
clearly. Since my copy of geronimo doesn't build without my changes
to tomcat/jasper, to proceed with this I need to get my version of
tomcat out somehow.
Here are some possibilities I've thought of, maybe someone can think
of something even better:
- attach my tomcat patch to a geronimo jira issue, maybe check the
patch into svn somewhere, build tomcat + jasper locally, and put them
into our "repository" module (mabye with source jars if I can figure
out how to create them)
- svn copy tomcat to our tree and apply the patch, check it in, and
push "org.apache.geronimo.tomcat/jasper" jars to the snapshot repo
In any case I'd prefer to check my geronimo changes into trunk in the
optimistic expectation that the Tomcat community will accept something
like this proposal and that if they don't it will still be easier to
adapt to the AnnotationProcessor approach in trunk than to deal with a
geronimo branch.
Anyway I started GERONIMO-3010 and I'll attach my tomcat patches there
even though they aren't quite ready to propose to the tomcat
community. (AFAIK they work but especially the jasper code generation
needs to be cleaned up)
Thoughts?
thanks
david jencks
--No virus found in this incoming message.
Checked by AVG Free Edition.
Version: 7.5.446 / Virus Database: 268.18.17/730 - Release Date:
3/22/2007 7:44 AM
