Great, I'll work on these changes today. Thanks much
David Blevins wrote:
On Feb 11, 2007, at 6:12 PM, Tim McConnell wrote:
HI Dain, thanks for reviewing. Yes, you're right--I am scanning all
the classes in the web module for annotations, which might be
excessive. I'll take a look at the OpenEJB AnnotationDeployer and
DeploymentLoader classes tomorrow and see if I can discern the
technique you mention (and then try to emulate it). If I have
questions, which is likely, I'll ask. Thanks much
It's pretty easy. Essentially I made it so ClassFinder can be
constructed with a var-arg list of classes (may also support a plain
List, can't remember). When that is done it only searches that set for
annotations and doesn't do the whole asm routine to come up with a list
of annotated classes.
And now that I mention it, if you were to shift to that style of
annotation searching you could disregard the comment I made earlier
about constructing the ClassFinder exactly once per codebase -- if you
could pretty much construct them and throw them away as you like.
As far as the patch, there's nothing in there that can't be easily fixed
and we're all on the same page, so I can check it in and you can work on
your updates or I can wait for a revised patch. Whichever is easiest
for you. Let me know.
-David
Dain Sundstrom wrote:
Are you sure you must scan all classes in the web module for @EJB
annotations? I don't think it is necessary or what you want to do.
I believe that you should only be checking specific classes for the
annotation such as all known servlets. This is how we process
annotations for EJBs. First we find all EJBs, either listed in the
ejb-jar.xml or found by scanning for @Stateless, @Stateful and
@MessageDriven. Once we have the list of EJBs we check each class
for the presence of the @EJB annotation.
In your case, I believe you are scanning for all classes that have
the @EJB annotation which will be a much larger number of classes
then just the servlets. This over processing of classes can easily
lead to naming conflicts. Also, I do not believe that servlet 2.5
has an @Servlet annotation, so I don't think you have to do any
general purpose scanning like EJB module must do, which should make
web deployment much more efficient.
Other than that, the patches look good :) One minor thing to note is
we don't use prefixes such as "m_" for variables.
Good job,
-dain
On Feb 10, 2007, at 1:23 AM, Tim McConnell wrote:
Hi, I've attached a patch and two new classes to the GERONIMO-2816
JIRA that provides support for the @EJB and @EJBs
annotations if anyone would like to review. The intent is to
demonstrate a final/permanent technique that can be extended and
used throughout Geronimo to support annotations for JSR-88. This
patch and new code works for Tomcat and circumvents the annotation
processing that is currently in EjbRefBuilder (which did not update
the deployment descriptor with the discovered annotations, but only
updated the JNDI references). I'd appreciate some feedback before I
start propagating this technique to the other module builders to
support the remainder of the annotations. I already have another
subclass ready to support the @Resource annotations but I didn't
want to include it in this example since it doesn't demonstrate
anything different than the EJBAnnotationHelper subclass, and I'd
like to get some feedback first on the the technique. The general
technique, which we've discussed before, is pretty straightforward
and is summarized here for reference:
1 -- Discover the annotations: I had hoped that we could centralize
the discovery of annotations in the Deployer class prior to the
createModule phase of deployment, but as David Blevins pointed out
this is almost impossible. So it has to be pushed down into the
installModule phase of deployment after the necessary classloader(s)
and module context(s) have been established (see the changes for
AbstractWebModuleBuilder for an example).
2 -- Process the annotations: This just means to update the existing
deployment descriptor (or create a new one) with the discovered
annotations.
3 -- Set metadata-complete in the deployment descriptor to prevent
repeated processing of annotations (see the EjbRefBuilder changes
for an example)
4 -- Update the deployment descriptor in the module so that it can
flow through the remainder of the deployment process much as before
(for JNDI naming and resolution, and to remain with module in
support of the JSR-77 requirements for management).
--
Thanks,
Tim McConnell
--Thanks,
Tim McConnell
--
Thanks,
Tim McConnell
