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