JIRA to track the bug - https://issues.apache.org/jira/browse/OPENEJB-584
Cheers Prasad On 3/24/07, Prasad Kashyap <[EMAIL PROTECTED]> wrote:
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 > > >> >> > >> > > >> >> > >> > > >> >> > >> > > >> >> > > > > >> >> > > > >> >> > > > >> >> > > >> > > > >> > > >> > > > > > > > >
