On Apr 6, 2007, at 2:54 AM, Remy Maucherat wrote:
David Jencks wrote:
i've worked on this some more and come up with a patch that I feel
more or less comfortable showing to tomcat :-) at https://
issues.apache.org/jira/browse/GERONIMO-3010 or directly https://
issues.apache.org/jira/secure/attachment/12355053/
GERONIMO-3010-3a.patch
Thanks.
I'm happy to open a bugzilla entry and attach the patch to it as
well, but I also need to keep the patch tracked in geronimo until
there's more resolution as to its fate.
This patch features:
-- xml override of annotations. There's even a manual test via a
new EnvEntryExample servlet which is present but not entirely
integrated into the examples webapp.
-- postCreate and preDestroy annotated methods are looked for on
all superclasses per spec.
-- Improved interface name of InstanceManager rather than
LifecycleProvider, thanks David Blevins
Ok.
-- Jasper does not use any Catalina classes, and catalina only
uses a jasper class if it creates its own InstanceManager: if you
supply an InstanceManager yourself there is no classes dependency
of catalina on jasper (although it appears that startup creates a
JspServlet in a default web app???). This removes the need for
sharing org.apache.AnnotationProcessor between catalina and jasper.
Submitting this in the patch makes me look at it and consider it,
but I much prefer keeping a shared interface between Catalina and
Jasper (because otherwise it's pretty obvious one of them becomes
dependent on the other; and predictably, the main instance manager
extends org.apache.jasper.instanceManagement.InstanceManager).
IIRC everyone who commented on my previous patch complained that I
had a shared class between catalina and jasper, so I changed it. If
tomcat and jasper were truly independent, with no shared classes at
all, you wouldn't be able to run jasper in tomcat without reflection
or something that served as a container for both. I think my current
implementation makes it clear why, as you note. I very slightly
prefer the current implementation because it really does make it
possible to run jasper in other servlet containers without any
leakage of catalina classes. However I really don't care much, and
am happy to go back to a single interface, but I won't put it in the
org.apache package.
Other smaller problems:
- InstanceManagerFactory in Jasper is a bit too convoluted as far
as I am concerned
I'll see if I can think up a simpler way to do this.
- I still don't know if I agree with removing the security checks
out of Catalina, merging checks between filters and servlets, etc
I think that the security checks and related code are really specific
to the tomcat classloader and you shouldn't force them on containers
embedding tomcat that use different classloaders. As far as
uniformizing the code between servlets, filters, and listeners, I
still don't understand the problem. You might well be right, but I'd
really appreciate concrete details on why some checks are appropriate
for servlets and not listeners or filters. Since all of these
objects are only created once (except for single-thread-model
servlets, which have all the checks anyway) I don't really see why a
slight difference in startup speed would outweigh the simplicity of
uniform code and relative ease of understanding of having the object
creation policy in one place.
- Class hierarchy for InstanceManager -> meh (although I suppose it
perfectly fits your needs, though ...)
I agree... Geronimo implements its own InstanceManagers for tomcat
and jasper, so we don't have that problem. I'd be perfectly happy to
drop support for AnnotationProcessor entirely, but I kinda doubt you
would like it so much :-)
-- jasper loads classes more directly, tomcat uses care in
figuring out if security precautions are needed and which
classloader should be used.
Some more improvements are possible, for example the jasper
generated classes are loaded twice. I'd rather make sure that the
current state doesn't break a lot of the tck before making a lot
more changes.
To be honest, this whole stuff may be 6.next material. We'll see.
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]
