Hello,
Please remove me from mailing list, Thanks in advance !!!
M Khurram The only way to contorl or finish successfully "Think before it will happen, as when it will be happening you will not have time to Think"
From: David Blevins <[EMAIL PROTECTED]>
Reply-To: [email protected]
To: [email protected]
Subject: Re: Wrong interceptor chaining order
Date: Thu, 22 Mar 2007 13:49:14 -0700
>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
>>> > >>
>>> > >>
>>> > >>
>>> > >
>>> >
>>> >
>>>
>>
>
Express yourself instantly with MSN Messenger! MSN Messenger Download today it's FREE!
