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
