0

I'm having a little trouble getting a logging aspect set up using SpringAOP + AspectJ. I would like an "Around" method to fire when either a class or method is annotated with the @Loggable annotation. Below is my advice code:

@Around(value = "execution( * *(..)) && target(bean) && @annotation(loggable)", argnames "bean, loggable")
public void test1(ProceedingJoinPoint method, Object bean, Loggable loggable) { }

@Around(value = "execution( * *(..)) && target(bean) && @within(loggable)", argnames "bean, loggable")
public void test2(ProceedingJoinPoint method, Object bean, Loggable loggable) { }

@Around(value = "execution( * *(..)) && target(bean) && (@annotation(loggable) || @within(loggable))", argnames "bean, loggable")
public void test3(ProceedingJoinPoint method, Object bean, Loggable loggable) { }

test1 and test2 fire. test3 does not, and it's the one that I really want. Any thoughts on why this might be?

kriegaex
  • 63,017
  • 15
  • 111
  • 202
VillagElliott
  • 31
  • 6

1 Answers1

1

First of all, there are syntax errors in your pointcuts. It is not lower-case argnames but argNames and you are missing an = in between parameter name and value. So it must be argNames = "bean, loggable".

Secondly if your advice returns void it will only match methods returning void as well. The more general case is to return Object in the advice to really match all methods.

Last but not least, you should see a warning which explains the problem with the third pointcut. This is displayed in your Eclipse IDE or on the AspectJ compiler's (ajc) log output:

ambiguous binding of parameter(s) loggable across '||' in pointcut

This means that you cannot say "bind one value or the other to the parameter 'loggable'". What if both conditions match? Which one should be assigned? You have two options, assuming your fully-qualified class name is de.scrum_master.app.Loggable:

A) No reference to @Loggable annotation needed:

This is the simple case. If @Loggable does not have any parameters you need to read, it is not necessary to bind it to a parameter. BTW, if you want your pointcut to also capture static methods, you should not bind target() either because the target would be null. Maybe in Spring-AOP this is irrelevant because it only works with Spring Beans anyway, but in full-featured AspectJ it would make a difference because it is more powerful.

@Around(value = "execution(* *(..)) && (@annotation(de.scrum_master.app.Loggable) || @within(de.scrum_master.app.Loggable))")
public Object withoutLoggableReference(ProceedingJoinPoint thisJoinPoint) {
    Object bean = thisJoinPoint.getTarget();
    System.out.println(thisJoinPoint + " -> " + bean);
    return thisJoinPoint.proceed();
}

Or, equivalently:

@Around(value = "execution(* (@de.scrum_master.app.Loggable *.*)(..)) || execution(@de.scrum_master.app.Loggable * *.*(..))")
public Object withoutLoggableReference(ProceedingJoinPoint thisJoinPoint) {
    Object bean = thisJoinPoint.getTarget();
    System.out.println(thisJoinPoint + " -> " + bean);
    return thisJoinPoint.proceed();
}

B) Reference to @Loggable annotation needed:

You have no other choice than to go with two separate pointcuts if you want to bind the annotations to parameters. Maybe you could use a utility method doing the actual logging in order to avoid code duplication in your advice.

kriegaex
  • 63,017
  • 15
  • 111
  • 202