Thanx.. https://issues.apache.org/jira/secure/attachment/12354016/AnnotationDeployer.patch
Cheers Prasad On 3/22/07, David Blevins <[EMAIL PROTECTED]> wrote:
Your patch got line wrapped. Here, I've reopened this issue, go ahead and throw in in there: http://issues.apache.org/jira/browse/OPENEJB-87 -David On Mar 22, 2007, at 1:21 PM, Prasad Kashyap wrote: > Index: container/openejb-core/src/main/java/org/apache/openejb/ > config/AnnotationDeployer.java > =================================================================== > --- container/openejb-core/src/main/java/org/apache/openejb/config/ > AnnotationDeployer.java (revision > 521254) > +++ container/openejb-core/src/main/java/org/apache/openejb/config/ > AnnotationDeployer.java (working > copy) > @@ -63,6 +63,7 @@ > import java.net.URL; > import java.util.ArrayList; > import java.util.Arrays; > +import java.util.Collection; > import java.util.List; > import java.util.Map; > > @@ -307,14 +308,15 @@ > > Interceptors interceptors = > clazz.getAnnotation(Interceptors.class); > if (interceptors != null){ > - EjbJar ejbJar = ejbModule.getEjbJar(); > + EjbJar ejbJar = ejbModule.getEjbJar(); > for (Class interceptor : interceptors.value()) { > if > (ejbJar.getInterceptor(interceptor.getName()) == null){ > ejbJar.addInterceptor(new > Interceptor(interceptor.getName())); > } > } > > - InterceptorBinding binding = > assemblyDescriptor.addInterceptorBinding(new InterceptorBinding()); > + InterceptorBinding binding = new > InterceptorBinding(); > + assemblyDescriptor.getInterceptorBinding().add > (0, binding); > binding.setEjbName(bean.getEjbName()); > > for (Class interceptor : interceptors.value()) { > @@ -332,7 +334,8 @@ > } > } > > - InterceptorBinding binding = > assemblyDescriptor.addInterceptorBinding(new InterceptorBinding()); > + InterceptorBinding binding = new > InterceptorBinding(); > + > assemblyDescriptor.getInterceptorBinding().add(0, binding); > binding.setEjbName(bean.getEjbName()); > > for (Class interceptor : interceptors.value > ()) { > > > > On 3/22/07, Prasad Kashyap <[EMAIL PROTECTED]> wrote: >> On 3/22/07, David Blevins <[EMAIL PROTECTED]> wrote: >> > >> > On Mar 22, 2007, at 8:35 AM, Prasad Kashyap wrote: >> > >> > > Are you sure we can't just change the spec instead ? >> > >> > Maybe last year at this time :) >> > >> > > This is much complicated than the "simple" change you >> mentioned. It >> > > affects both lines 317 (class-level interceptor) and 335 >> (method-level >> > > interceptor). >> > > >> > > InterceptorBinding binding = >> > > assemblyDescriptor.addInterceptorBinding(new InterceptorBinding >> ()); >> > > >> > > addInterceptorBinding() gets the existing binding list and >> adds one to >> > > the bottom. >> > > >> > > Now if you simply modify this to add one to the top of the >> list, then >> > > it would reverse the specification order as defined by the the >> > > annotations in the class. That's a no-no. >> > > >> > > The right way to do this would be to >> > > 1. go over the collection of @Interceptor annotatations in the >> > > reverse order >> > > 2. insert the interceptor-binding to the top of the >> bindingsList in >> > > the assembly-descriptor. >> > > >> > >> > I mean at line 317 only (class-level interceptor) make a call like >> > this instead: >> > >> > IntercpetorBinding binding = new IntercpetorBinding() >> > assemblyDescriptor.getInterceptorBinding().add(0, binding); >> > >> > This would put only the annotated class-level binding before all >> the >> > other class-level bindings for that bean. It technically puts it >> > before all bindings, but they get resorted into package, class, and >> > method order anyway, so it works out fine. >> > >> > We wouldn't need to go over the @Interceptor annotation in reverse >> > order as a class can only have one @Interceptor annotation, so we >> > should be good as the loop will only ever execute once. >> >> >> Oh shucks. I knew that. While reading the code, I had the >> @Interceptors confused with the interceptors.value(). I don't know >> how that confusion crept in. Thanks for patiently clearing that up >> for me. >> >> Adding the binding to the top of the list does work. >> >> I think we should do the same for the method-level bindings too >> (line 335). >> >> Cheers >> Prasad >> >> > >> > Thoughts? >> > >> > -David >> > >> > >> > > >> > > On 3/21/07, David Blevins <[EMAIL PROTECTED]> wrote: >> > >> Wow, you're going to be the interceptor king pretty soon here :) >> > >> This is an extremely sharp observation. I certainly never >> saw it. >> > >> >> > >> Comments below... >> > >> >> > >> On Mar 21, 2007, at 12:37 PM, Prasad Kashyap wrote: >> > >> >> > >> > When an interceptor is declared in the DD and bound >> specifically >> > >> to a >> > >> > bean (not using the wildcard *), it becomes yet another class >> > >> > interceptor for the bean. In such a scenario, Section >> 12.8.2 of the >> > >> > spec says, >> > >> > >> > >> > "The binding of interceptors to classes is additive. If >> > >> interceptors >> > >> > are bound at the class-level and/or default-level as well >> as at the >> > >> > method-level, both class-level and/or default-level as well as >> > >> > method-level interceptors will apply. The deployment >> descriptor >> > >> may be >> > >> > used to augment the interceptors and interceptor methods >> defined by >> > >> > means of annotations. >> > >> > >> > >> > <emphasis>When the deployment descriptor is used to augment >> the >> > >> > interceptors specified in annotations, the interceptor methods >> > >> > specified in the deployment descriptor will be invoked >> after those >> > >> > specified in annotations, </emphasis> >> > >> > >> > >> > according to the ordering specified in sections 12.3.1 and >> > >> 12.4.1. The >> > >> > interceptor-order deployment descriptor element may be used to >> > >> > override this ordering." >> > >> > >> > >> > Expected outcome: >> > >> > ----------------------------- >> > >> > As per the statement in <emphasis></emphasis> above, the >> > >> interceptor >> > >> > declared in the DD and bound to the bean should be invoked >> after >> > >> all >> > >> > the other class level interceptors specified by annotations. >> > >> This is >> > >> > assuming that the order is not modified by the <interceptor- >> order> >> > >> > element. >> > >> > >> > >> > Actual result: >> > >> > ------------------- >> > >> > The actual result I'm seeing is the class level interceptor >> > >> bound in >> > >> > the DD gets invoked by before those specified by the >> @Interceptor >> > >> > annotation. >> > >> >> > >> This is easy to fix, we just need to tweak the >> AnnotationDeployer to >> > >> put the interceptor declarations found via the @Interceptors >> > >> annotation at the *beginning* of the list rather than at the >> end. >> > >> >> > >> http://fisheye6.cenqua.com/browse/openejb/trunk/openejb3/ >> container/ >> > >> openejb-core/src/main/java/org/apache/openejb/config/ >> > >> AnnotationDeployer.java?r=519454#l335 >> > >> >> > >> So right there on line 335 (love fisheye) we'd need to put that >> > >> binding *before* the bindings already declared in the list. >> > >> >> > >> Feel free to hack on it. It's actually a fantastic place to >> starting >> > >> digging into the runtime code. >> > >> >> > >> -David >> > >> >> > >> >> > >> >> > > >> > >> > >> >
