geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Blevins <david.blev...@visi.com>
Subject Re: GERONIMO-2816 patch
Date Sat, 10 Feb 2007 21:54:16 GMT

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
>


Mime
View raw message