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 Mon, 12 Feb 2007 04:53:39 GMT

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
>


Mime
View raw message