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 02:12:05 GMT
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

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