1

I have such aspect-based logging:

@Pointcut("@annotation(Loggable)")
public void loggableAnnotation() {}

@Around("loggableAnnotation()")
public Object simpleProcess(ProceedingJoinPoint joinPoint) throws Throwable {
  return this.processWithBody(joinPoint, null);
}

@Around("loggableAnnotation() && args(@org.springframework.web.bind.annotation.RequestBody body,..)")
public Object processWithBody(ProceedingJoinPoint joinPoint, Object body) throws Throwable {
  // do things
}

And it works fine when I perform request with @RequestBody, advice processWithBody() is triggered. But when I perform request that doesn't have a @RequestBody (only @PathVariable and @RequestParam) the simpleProcess() is not triggered, instead in the processWithBody() I receive path variable value as the body parameter.

Why does it happen and how do I pocess two types of requests differently (in the same advice if possible)?

kriegaex
  • 63,017
  • 15
  • 111
  • 202
Svetlana Guma
  • 37
  • 1
  • 7
  • 1
    Plese share code/details of the methods that are adviced as well. – R.G Apr 18 '20 at 02:34
  • I agree that you should share an [MCVE](https://stackoverflow.com/help/mcve), i.e. full aspect, application and annotation classes including package names and imports. For example, if `Loggable` does not happen to be in the same package as the aspect, it will not be found at all because you don't use a fully qualified class name like `@annotation(my.package.Loggable)`. – kriegaex Apr 18 '20 at 04:35

1 Answers1

2

You are making three basic miskates:

  • You are trying to match parameter argument annotations from within args(), but there it has no effect, which is why processWithBody(..) matches an unwanted parameter and binds it to body. It should be transferred to an execution() pointcut.

  • Your pointcut syntax is wrong, even if you transfer it to execution(), i.e. something like
    execution(* *(@org.springframework.web.bind.annotation.RequestBody *, ..)) would match if the type(!) of the parameter has a @RequestBody annotation, not the parameter itself.
    In order to achieve that you need to put the parameter itself into parentheses like (*), i.e.
    execution(* *(@org.springframework.web.bind.annotation.RequestBody (*), ..)).

  • You have to make sure that the pointcuts are mutually exclusive, otherwise multiple advices shall match on the same joinpoint. To be precise, you need to differentiate between the following cases:

    • methods annotated by @Loggable with a first method parameter annotated by @RequestBody
    • methods annotated by @Loggable with a first method parameter not annotated by @RequestBody
    • methods annotated by @Loggable without any parameters

Here is an example in plain Java + AspectJ (no Spring or Spring AOP), but the aspect syntax should be identical in Spring AOP:

Annotation + driver application:

package de.scrum_master.app;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Retention;
import java.lang.annotation.Target;

@Retention(RUNTIME)
@Target(METHOD)
public @interface Loggable {}
package de.scrum_master.app;

import org.springframework.web.bind.annotation.RequestBody;

public class Application {
  public static void main(String[] args) {
    Application application = new Application();
    application.doNotLogMe("foo", 11);
    application.doNotLogMeEither();
    application.doNotLogMeEither("foo", 11);
    application.logMe("foo", 11);
    application.logMeToo("foo", 11);
    application.logMeToo();
  }

  public void doNotLogMe(@RequestBody String body, int number) {}
  public void doNotLogMeEither() {}
  public void doNotLogMeEither(String body, int number) {}
  @Loggable public void logMe(@RequestBody String body, int number) {}
  @Loggable public void logMeToo(String body, int number) {}
  @Loggable public void logMeToo() {}
}

Aspect:

As you can see, I am using the differentiating the three cases mentioned above and also satisfy your need for a common helper method which I called logIt(..). There you can put all the complex logging stuff you want to use without having any duplicate code in your advice methods.

package de.scrum_master.aspect;

import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Pointcut;

@Aspect
public class MyAspect {
  @Pointcut("@annotation(de.scrum_master.app.Loggable)")
  public void loggableAnnotation() {}

  @Around(
    "loggableAnnotation() && " + 
    "execution(* *())"
  )
  public Object simpleProcessWithoutParameters(ProceedingJoinPoint joinPoint) throws Throwable {
    return logIt(joinPoint, null);
  }

  @Around(
    "loggableAnnotation() && " +
    "execution(* *(!@org.springframework.web.bind.annotation.RequestBody (*), ..))"
  )
  public Object simpleProcessWithParameters(ProceedingJoinPoint joinPoint) throws Throwable {
    return logIt(joinPoint, null);
  }

  @Around(
    "loggableAnnotation() && " + 
    "execution(* *(@org.springframework.web.bind.annotation.RequestBody (*), ..)) && " +
    "args(body, ..)"
  )
  public Object processWithBody(ProceedingJoinPoint joinPoint, Object body) throws Throwable {
    return logIt(joinPoint, body);
  }

  private Object logIt(ProceedingJoinPoint joinPoint, Object body) throws Throwable {
    System.out.println(joinPoint + " -> " + body);
    return joinPoint.proceed();
  }
}

Console log:

execution(void de.scrum_master.app.Application.logMe(String, int)) -> foo
execution(void de.scrum_master.app.Application.logMeToo(String, int)) -> null
execution(void de.scrum_master.app.Application.logMeToo()) -> null

P.S.: The difference between execution(* *(@MyAnn *)) and execution(* *(@MyAnn (*))) is subtle and thus tricky. Unfortunately, it is not properly documented here where it should be. To be precise, the latter case is not documented at all, only maybe in some AspectJ release notes and of course in unit tests. But no normal user would look there.

kriegaex
  • 63,017
  • 15
  • 111
  • 202
  • thank you! I supposed that pointcuts should be exclusive (also found out that you can't put '||' in the advice so that the bodyless method had `null` in the `args(body)`) but had trobles figuring out where to put operator `!` – Svetlana Guma Apr 18 '20 at 13:16
  • Yes, with method parameter binding via `args()`, `target()`, `this()`, `@annotation()` etc. you cannot use `||` because then the binding could be ambiguous and so it is considered to be a syntax error by the AspectJ pointcut parser. – kriegaex Apr 18 '20 at 14:07