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 >> >> >> >
