You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by David Jencks <da...@yahoo.com> on 2007/04/06 11:03:21 UTC
Re: Annotation processing - Geronimo injection
On Mar 27, 2007, at 8:31 PM, David Jencks wrote:
>> <big snip>
>> Right now, it's mostly pita-win (it's a significant
>> refactoring) :D You should IMO offer some incentive as part of
>> this to justify the refactoring, such as support for web.xml
>> annotation overrides in standalone Tomcat (as you can see, there's
>> full support for annotations, but not overriding).
>
> i'll look into this... although I don't see the code involved as
> being very connected to what I've been proposing.
>
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
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
-- 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.
-- 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.
Comments?
thanks
david jencks
Re: Annotation processing - Geronimo injection
Posted by Remy Maucherat <re...@apache.org>.
David Blevins wrote:
>
> On Apr 6, 2007, at 5:03 AM, Remy Maucherat wrote:
>
>> David Jencks wrote:
>>> but I won't put it in the org.apache package.
>>
>> There are not too many solutions to this problem, and (unfortunately
>> for you, apparently) I think it's the most appropriate, or least
>> inappropriate if you prefer.
>
> Just as a note, I'd be great if people didn't put classes directly in
> the org.apache namespace. If every java project at Apache did that,
> we'd have a mess real quick.
I agree with that, it is really a last resort solution.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by David Blevins <da...@visi.com>.
On Apr 6, 2007, at 5:03 AM, Remy Maucherat wrote:
> David Jencks wrote:
>> but I won't put it in the org.apache package.
>
> There are not too many solutions to this problem, and
> (unfortunately for you, apparently) I think it's the most
> appropriate, or least inappropriate if you prefer.
Just as a note, I'd be great if people didn't put classes directly in
the org.apache namespace. If every java project at Apache did that,
we'd have a mess real quick.
-David
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by Remy Maucherat <re...@apache.org>.
David Jencks wrote:
> I haven't done any speed measurements yet, have you?
It does have an impact.
I have added a system property to allow specifying if the instance
manager should be used to create tag instances (default to not using it).
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by David Jencks <da...@yahoo.com>.
On May 24, 2007, at 10:10 AM, Remy Maucherat wrote:
> David Jencks wrote:
>> I tend to agree that a new branch is more appropriate. However,
>> earlier versions of the patch had a compatibility layer so the old
>> api could continue to work. I'm happy to add that back in on
>> request.
>
> I am examining the patch in detail as part of the new trunk branch
> (adjusting things like package names), and unfortunately it turns
> out I still dislike it due to design implications with Jasper.
>
> Modern JSPs now uses lots of tags, and especially lots of
> SimpleTags, which are not pooled. As a result, the InstanceManager
> design leads to replacing "new MySimpleTag()" with "newInstance
> (String className)" (which creates the instance in a much more
> expensive way). Maybe adding access to InstanceManager.newInstance
> (Object instance) would be a solution (in effect making
> InstanceManager an extension of the old AnnotationProcessor).
To reorient myself to this.... these tags can have injected stuff,
correct? So it would theoretically be possible to support
constructor injection on them, with suitable constructor metadata?
Are tags typically created once per jsp instance or once per request
(or at some other frequency)?
And your objection here is that
new Foo();
is sufficiently faster than
Class fooClass = cl.loadClass("Foo");
Object foo = fooClass.newInstance();
that we should separate the object construction and injection phases?
My thoughts at this time are that:
- if tags are created once per jsp we're only talking a difference in
startup time which is not important
- it might be worth eliminating the loadClass call in any case
- if tags use injection the cost of looking up the stuff to inject so
far dwarfs the time to create the object that the single
InstanceManager.newInstance method still makes sense
- if a tag doesn't use injection and is created once per request (or
more often) then it would be worth measuring the speed difference and
possibly modifying the code generator to directly create the tags
when there are no injections for it.
I haven't done any speed measurements yet, have you?
thanks
david jencks
>
> Rémy
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by Remy Maucherat <re...@apache.org>.
David Jencks wrote:
> I tend to agree that a new branch is more appropriate. However, earlier
> versions of the patch had a compatibility layer so the old api could
> continue to work. I'm happy to add that back in on request.
I am examining the patch in detail as part of the new trunk branch
(adjusting things like package names), and unfortunately it turns out I
still dislike it due to design implications with Jasper.
Modern JSPs now uses lots of tags, and especially lots of SimpleTags,
which are not pooled. As a result, the InstanceManager design leads to
replacing "new MySimpleTag()" with "newInstance(String className)"
(which creates the instance in a much more expensive way). Maybe adding
access to InstanceManager.newInstance(Object instance) would be a
solution (in effect making InstanceManager an extension of the old
AnnotationProcessor).
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by David Jencks <da...@yahoo.com>.
On May 17, 2007, at 2:41 AM, Remy Maucherat wrote:
> David Jencks wrote:
>> As far as I know I've addressed all the issues that were raised,
>> but that might have a tenuous relationship to whether I actually
>> did :-). I'd certainly appreciate review by tomcat committers
>> before I completely forget how my patch works :-)
>
> The patch could see minor modifications when it is committed, but I
> am not going to ask you to do them. As for the actual commit, the
> problem is I don't know if it's appropriate to commit this in 6.0.x
> rather than in a new branch (at the moment, I would say no, as at
> least one project uses the API).
I tend to agree that a new branch is more appropriate. However,
earlier versions of the patch had a compatibility layer so the old
api could continue to work. I'm happy to add that back in on request.
thanks
david jencks
>
> Rémy
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by Remy Maucherat <re...@apache.org>.
David Jencks wrote:
> As far as I know I've addressed all the issues that were raised, but
> that might have a tenuous relationship to whether I actually did :-).
> I'd certainly appreciate review by tomcat committers before I completely
> forget how my patch works :-)
The patch could see minor modifications when it is committed, but I am
not going to ask you to do them. As for the actual commit, the problem
is I don't know if it's appropriate to commit this in 6.0.x rather than
in a new branch (at the moment, I would say no, as at least one project
uses the API).
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by David Jencks <da...@yahoo.com>.
On May 16, 2007, at 1:10 PM, Paul McMahan wrote:
> On Apr 12, 2007, at 12:15 PM, Remy Maucherat wrote:
>
>> David Jencks wrote:
>>> https://issues.apache.org/jira/secure/attachment/12355273/
>>> GERONIMO-3010-4.patch In addition, this one combines the
>>> InstanceManager interfaces. I think this is a bad idea because
>>> it forces jasper to use an interface shared with catalina, which
>>> the previous patch does not. This makes catalina marginally more
>>> complicated when used as a standalone container for jasper but
>>> makes it possible to use jasper outside catalina without any
>>> catalina classes. Note also that the previous (3c) patch allows
>>> use of tomcat embedded in a container that supplies its own
>>> InstanceManager implementation without pulling in any jasper
>>> classes at runtime.
>>> This has 1 interface, 1 implementation.
>>
>> Thanks for the patch revisions. I will look into adding these
>> features after the next stable release.
>>
>> Rémy
>
> Now that 6.0.13 has been released is it a good time to resume this
> discussion about annotation processing? I have been using David's
> patch in geronimo's Tomcat assembly for the last several weeks and
> it has proven to be quite stable. David, do you consider the
> latest patch to be feature complete as far as where the discussion
> left off? Have the issues that were raised in this discussion been
> addressed?
As far as I know I've addressed all the issues that were raised, but
that might have a tenuous relationship to whether I actually
did :-). I'd certainly appreciate review by tomcat committers before
I completely forget how my patch works :-)
thanks
david jencks
>
>
> Best wishes,
> Paul
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by Paul McMahan <pa...@gmail.com>.
On Apr 12, 2007, at 12:15 PM, Remy Maucherat wrote:
> David Jencks wrote:
>> https://issues.apache.org/jira/secure/attachment/12355273/
>> GERONIMO-3010-4.patch In addition, this one combines the
>> InstanceManager interfaces. I think this is a bad idea because it
>> forces jasper to use an interface shared with catalina, which the
>> previous patch does not. This makes catalina marginally more
>> complicated when used as a standalone container for jasper but
>> makes it possible to use jasper outside catalina without any
>> catalina classes. Note also that the previous (3c) patch allows
>> use of tomcat embedded in a container that supplies its own
>> InstanceManager implementation without pulling in any jasper
>> classes at runtime.
>> This has 1 interface, 1 implementation.
>
> Thanks for the patch revisions. I will look into adding these
> features after the next stable release.
>
> Rémy
Now that 6.0.13 has been released is it a good time to resume this
discussion about annotation processing? I have been using David's
patch in geronimo's Tomcat assembly for the last several weeks and it
has proven to be quite stable. David, do you consider the latest
patch to be feature complete as far as where the discussion left
off? Have the issues that were raised in this discussion been
addressed?
Best wishes,
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by Remy Maucherat <re...@apache.org>.
David Jencks wrote:
> Excellent, thanks! Would this be shortly after 6.0.11 is out or further
> in the future? If you have any guess about a date it would be much
> appreciated.
No idea. For example, I thought 6.0.11 would be ok, but it's not. I
don't know if I'll commit as is either, since I did not look at the
revised patches.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by David Jencks <da...@yahoo.com>.
On Apr 12, 2007, at 9:15 AM, Remy Maucherat wrote:
> David Jencks wrote:
>> https://issues.apache.org/jira/secure/attachment/12355273/
>> GERONIMO-3010-4.patch In addition, this one combines the
>> InstanceManager interfaces. I think this is a bad idea because it
>> forces jasper to use an interface shared with catalina, which the
>> previous patch does not. This makes catalina marginally more
>> complicated when used as a standalone container for jasper but
>> makes it possible to use jasper outside catalina without any
>> catalina classes. Note also that the previous (3c) patch allows
>> use of tomcat embedded in a container that supplies its own
>> InstanceManager implementation without pulling in any jasper
>> classes at runtime.
>> This has 1 interface, 1 implementation.
>
> Thanks for the patch revisions. I will look into adding these
> features after the next stable release.
Excellent, thanks! Would this be shortly after 6.0.11 is out or
further in the future? If you have any guess about a date it would
be much appreciated.
thanks
david jencks
>
> Rémy
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by Remy Maucherat <re...@apache.org>.
David Jencks wrote:
> https://issues.apache.org/jira/secure/attachment/12355273/GERONIMO-3010-4.patch
>
> In addition, this one combines the InstanceManager interfaces. I think
> this is a bad idea because it forces jasper to use an interface shared
> with catalina, which the previous patch does not. This makes catalina
> marginally more complicated when used as a standalone container for
> jasper but makes it possible to use jasper outside catalina without any
> catalina classes. Note also that the previous (3c) patch allows use of
> tomcat embedded in a container that supplies its own InstanceManager
> implementation without pulling in any jasper classes at runtime.
>
> This has 1 interface, 1 implementation.
Thanks for the patch revisions. I will look into adding these features
after the next stable release.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by David Jencks <da...@yahoo.com>.
I've come up with a couple more patches that try to address your
concerns:
https://issues.apache.org/jira/secure/attachment/12355259/
GERONIMO-3010-3c.patch
This one eliminates backwards compatibility support for
AnnotationProcessor and eliminates the replaceable aspects of jasper
InstanceManagerFactory, just leaving a static method for obtaining
the InstanceFactory. This is decidedly less flexible than the
previous implementation and now requires jasper to be run in a
container that supplies an InstanceManager. There are still separate
interfaces for InstanceManagers for catalina and jasper. I think
this is a considerably superior design than the next one...
This has 2 interfaces, 1 implementation (in tomcat, there's an unused
impl in jasper, see below).
https://issues.apache.org/jira/secure/attachment/12355273/
GERONIMO-3010-4.patch
In addition, this one combines the InstanceManager interfaces. I
think this is a bad idea because it forces jasper to use an interface
shared with catalina, which the previous patch does not. This makes
catalina marginally more complicated when used as a standalone
container for jasper but makes it possible to use jasper outside
catalina without any catalina classes. Note also that the previous
(3c) patch allows use of tomcat embedded in a container that supplies
its own InstanceManager implementation without pulling in any jasper
classes at runtime.
This has 1 interface, 1 implementation.
Both of these include a jasper DefaultInstanceManager that isn't
used, but could be by a project that wants to install jasper with no
annotation support.
I'd like to explore exactly how jasper creates classloaders and
whether its possible to use class reloading features of modern jvms
to eliminate the need to allow replacing the classloader in the
jasper InstanceManager at runtime. Unfortunately I doubt I'll have
time for this in the next month or so.
On Apr 6, 2007, at 5:03 AM, Remy Maucherat wrote:
> David Jencks wrote:
>> but I won't put it in the org.apache package.
>
> There are not too many solutions to this problem, and
> (unfortunately for you, apparently) I think it's the most
> appropriate, or least inappropriate if you prefer. I was already
> aware it was possible to have one interface in Jasper and implement
> it in Catalina, or one interface in Catalina and use it in Jasper,
> and it would work ;)
yes, and I prefer 2 interfaces with the possiblity of supplying
separate implementations. This makes catalina's standalone life
slightly more complicated but jasper's life outside catalina simpler
IMO.
>
>>> - 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.
>
> I don't know for sure about that, I would have to look at what gets
> run in each case. Maybe the implementation in the proposed patch is
> fine.
I could have missed something, but so far I think what I'm proposing
is OK.
>
>>> - 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 :-)
>
> It does not make any difference, really: if the API changes
> significantly, there's no need to keep the old API and provide fake
> compatibility, it would not really help IMO.
>
> I was complaining about the 2 interfaces + 1 abstract + 2 impls + 1
> factory (maybe I forgot some). This is overengineered (for Tomcat
> at least) IMO. Personally, the thing that I would like would be at
> most: 1 shared interface, 1 abstract, 1 impl. OTOH, I think I
> accept the idea of the InstanceManager interface.
>
> Overall, thanks for the patch, it gives a very good basis for work
> (esp if you did test it).
>
>>> To be honest, this whole stuff may be 6.next material. We'll see.
>> thanks!
>
> The first decision is to see if the API change is going to be
> acceptable in 6.0.x.
Looking forward to your comments...
>
> Rémy
thanks
david jencks
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by Remy Maucherat <re...@apache.org>.
David Jencks wrote:
> but I won't put it in the org.apache package.
There are not too many solutions to this problem, and (unfortunately for
you, apparently) I think it's the most appropriate, or least
inappropriate if you prefer. I was already aware it was possible to have
one interface in Jasper and implement it in Catalina, or one interface
in Catalina and use it in Jasper, and it would work ;)
>> - 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.
I don't know for sure about that, I would have to look at what gets run
in each case. Maybe the implementation in the proposed patch is fine.
>> - 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 :-)
It does not make any difference, really: if the API changes
significantly, there's no need to keep the old API and provide fake
compatibility, it would not really help IMO.
I was complaining about the 2 interfaces + 1 abstract + 2 impls + 1
factory (maybe I forgot some). This is overengineered (for Tomcat at
least) IMO. Personally, the thing that I would like would be at most: 1
shared interface, 1 abstract, 1 impl. OTOH, I think I accept the idea of
the InstanceManager interface.
Overall, thanks for the patch, it gives a very good basis for work (esp
if you did test it).
>> To be honest, this whole stuff may be 6.next material. We'll see.
>
> thanks!
The first decision is to see if the API change is going to be acceptable
in 6.0.x.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by David Jencks <da...@yahoo.com>.
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: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Annotation processing - Geronimo injection
Posted by Remy Maucherat <re...@apache.org>.
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).
Other smaller problems:
- InstanceManagerFactory in Jasper is a bit too convoluted as far as I
am concerned
- I still don't know if I agree with removing the security checks out of
Catalina, merging checks between filters and servlets, etc
- Class hierarchy for InstanceManager -> meh (although I suppose it
perfectly fits your needs, though ...)
> -- 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.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org