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