geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim McConnell <tim.mcco...@gmail.com>
Subject Re: GERONIMO-2816 patch
Date Mon, 12 Feb 2007 01:39:34 GMT
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

Mime
View raw message