Hi David, thanks for reviewing:
-- Yes we should be able to eliminate the annotation processing method in
EjbRefBuidler as it will now get a metadata-complete deployment descriptor.
I just didn't feel comfortable doing that for this review patch, but I can
do it in the patch that gets committed.
-- I haven't quite yet settled on the final demarcation of the annotations
that will be supported by each AnnotationHelper subclass, but I'm hoping I
can do it in such a manner that the classfinder would be instantiated only
once per builder. That may be impractical though, and I really like the two
alternatives you mention. So, I'll consider each as I encapsulate more
annotation functions.
Thanks
David Blevins wrote:
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).
Hey Tim, very clean and well documented code. I think we'll have to
strip out all the documentation to comply with Geronimo's strict no
javadoc policy ;)
Couple notes. We should be able to get rid of the @EJB for Web
processing in the EjbRefBuilder, no? That was there as temporary hack
and wasn't meant to last. You definitely have all the right code in the
EJBAnnotationHelper for Web/@EJB processsing.
Other note is you definitely want to construct the ClassFinder exactly
once for a module as each time it's constructed we'll have to reparse
all the class definitions in the entire module which is especially nasty
for web modules as there tend to be a lot of libraries in WEB-INF/lib
and WEB-INF/classes. I mention that as a cautionary note as from the
looks of where you're going each AnnotationHelper would create it's own
finder in it's constructor. We may want to create the ClassFinder and
pass it into the AnnotationHelpers and/or put it in the Module somewhere
for all to use if they need it.
-David
--
Thanks,
Tim McConnell
--
Thanks,
Tim McConnell
