AspectJ trouble using around advice and ProceedingJoinPoint

I'm new to AOP and I need to use AspectJ on my project. I need to use around advice but I have a problem using it, I've the following code in my .aj class,

pointcut checkUser(ProceedingJoinPoint jp,User user): call(* com.example.UserAccount.MyUI.checkUser(..))&& args(jp,user);

void around(ProceedingJoinPoint jp,User user) throws Throwable : checkUser(jp,user){
    // Condition checks one of the user boolean property
    if(condition){
        jp.proceed();
    }else{
        // Do nothing
    }   
}

but I get this warning all the time,

advice defined in Aspects.UserAccount has not been applied [Xlint:adviceDidNotMatch]

By the way, I tried it without ProceedingJoinPoint and tried just proceed(); but then got this warning, too few arguments to proceed, expected 1

I'm thankful for any single help or hint!

Reza

Solution

First I recommend to read the AspectJ documentation in order to learn the syntax. As you are using native AspectJ syntax, this is like learning a new programming language or at least a Java extension. What you are doing is mix native syntax with annotation-based syntax. Try to stick with one. I am sure that you did not find this in any tutorial but ended up with that syntax via trial and error.

You do not need to bind a joinpoint parameter in native syntax because it is there implicitly and automatically. The automatically bound joinpoint is always named thisJoinPoint as all tutorials surely show you. Only in annotation-based syntax you need to bind the joinpoint and can name it as you wish, but even then I recommend to stick with thisJoinPoint because then refactoring from annotation to native syntax is easier and your eyes get used to spotting that variable name in your aspect code.

The warning you get means that the pointcut you defined does not match any part of your code, at least not any part which is visible to the aspect weaver or compiler. There could be plenty of reasons why this can occur, e.g. misspelled package or class names, wrong around advice return type (return type must be Object for non-void methods or more specifically match what the method you want to intercept returns). Assuming that e.g. checkUser(..) returns a boolean, the around advice should do the same. I made up an example using your package and class names. Besides, package names should be lower-case but I used yours, assuming they are really package names and not inner classes:

Helper class:

package com.example.UserAccount;

public class User {
  private String name;

  public User(String name) {
    this.name = name;
  }

  public String getName() {
    return name;
  }

  @Override
  public String toString() {
    return "User(" + name + ")";
  }
}

Class targeted by aspect + sample main method:

package com.example.UserAccount;

public class MyUI {
  public boolean checkUser(User user) {
    return user.getName().toUpperCase().contains("ADMIN");
  }

  public static void main(String[] args) {
    MyUI ui = new MyUI();
    System.out.println(ui.checkUser(new User("Administrator")));
    System.out.println(ui.checkUser(new User("john")));
    System.out.println(ui.checkUser(new User("xander")));
    System.out.println(ui.checkUser(new User("admiral")));
    System.out.println(ui.checkUser(new User("SySaDmiN")));
  }
}

As you can see, we expect an output of "true" for the first and last entry, but "false" for the ones in between due to the check logic I made up for checkUser(..).

Now let us write an aspect which also returns "true" for a user named "Xander", e.g. in order to give him admin rights or whatever. I am making this up because you did not provide an MCVE as you always should on StackOverflow, but just an incoherent code snippet which keeps everyone trying to answer your question guessing what the heck you might want to achieve and how to reproduce your problem.

Aspect:

package Aspects;

import com.example.UserAccount.User;
import com.example.UserAccount.MyUI;

public aspect UserAccount {
  pointcut checkUser(User user) :
    execution(boolean MyUI.checkUser(*)) && args(user);

  boolean around(User user) : checkUser(user) {
    System.out.println(thisJoinPoint + " -> " + user);
    if (user.getName().equalsIgnoreCase("xander"))
      return true;
    return proceed(user);
  }
}

I just imported the MyUI class, so there is no need to use a fully-qualified class name here. Again, this is an advantage of native syntax, in annotation-based syntax you would have to use the fully qualified name.

I also replaced the generic * MyUI.checkUser(..) (which would also work) by the more explicit boolean MyUI.checkUser(*) because we already know that the method returns a boolean and has exactly one parameter, which both we assume anyway by returning a boolean from the around advice and by binding exactly one parameter via args(). You could also be even more specific and use boolean MyUI.checkUser(User).

Furthermore, I am using execution() rather than call() because it is more efficient, as it weaves the advice code just into the executing method once instead of five times for each method call in the main method. You only need to use call() if the MyUI class is out of reach of the AspectJ weaver/compiler, i.e. because it is not in the module you compile with AspectJ Maven.

Console log:

execution(boolean com.example.UserAccount.MyUI.checkUser(User)) -> User(Administrator)
true
execution(boolean com.example.UserAccount.MyUI.checkUser(User)) -> User(john)
false
execution(boolean com.example.UserAccount.MyUI.checkUser(User)) -> User(xander)
true
execution(boolean com.example.UserAccount.MyUI.checkUser(User)) -> User(admiral)
false
execution(boolean com.example.UserAccount.MyUI.checkUser(User)) -> User(SySaDmiN)
true

Et voilà, the aspect works. It makes the target method return "true" for user "xander".