https://issues.apache.org/jira/secure/attachment/12354131/Interceptor-v4.patch
https://issues.apache.org/jira/browse/OPENEJB-201 Cheers Prasad On 3/23/07, Prasad Kashyap <[EMAIL PROTECTED]> wrote:
This still does not seem to work for method-level interceptors. I have 2 method-level interceptors; 1 defined using annotation and 1 specified in the DD. As per the fix we applied recently, I am able to verify that the binding from the annotation is added to the list before the binding from the DD. Yet, the interceptor from the DD gets invoked before the interceptor from the annotation ! Cheers Prasad On 3/22/07, David Blevins <[EMAIL PROTECTED]> wrote: > > On Mar 22, 2007, at 6:44 PM, Prasad Kashyap wrote: > > > Thanx.. > > > > https://issues.apache.org/jira/secure/attachment/12354016/ > > AnnotationDeployer.patch > > > > Applied! Thank you very much! > > -David > > > > > 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 > >> >> > >> > >> >> > >> > >> >> > >> > >> >> > > > >> >> > > >> >> > > >> >> > >> > > >> > >> > > > >
