javaaopaspectjpointcut

aspectj pointcut for inherited interface methods

I want to intercept all java.sql.DataSource.getConnection methods with aspectj, I used this pointcut:

"execution(public java.sql.Connection javax.sql.DataSource+.getConnection(..))"

it works fine. but i encounter some classes ,for example org.apache.tomcat.jdbc.pool.DataSource that are implemented in a class hierarchy that this pointcut doesn't work, where the DataSource methods are in a class in the hierarchy that does not implement DataSource,only the top most class implements DataSource:

class BaseDataSource {

    public Connection getConnection() throws SQLException {
        return null;
    }


    public Connection getConnection(String username, String password) throws SQLException {
        return null;
    }

   implements all DataSource Methods...
}

class MyDataSource extends BaseDataSource implements java.sql.DataSource{
           //does not implement DataSource methods
}

BaseDataSource does not implement DataSource but has all the DataSource methods implementation.

the only pointcut i found that works is this:

execution(public java.sql.Connection *.getConnection(..)) && target(javax.sql.DataSource)

my question if there is a better way and if this pointcut might be worst with performance?

Solution

  • I replicated your situation in an MCVE as follows:

    Base class implementing DataSource methods, but not the interface:

    package de.scrum_master.app;

import java.io.PrintWriter;
import java.sql.Connection;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.util.logging.Logger;

public class BaseClass {
  public PrintWriter getLogWriter() throws SQLException { return null; }
  public void setLogWriter(PrintWriter out) throws SQLException {}
  public void setLoginTimeout(int seconds) throws SQLException {}
  public int getLoginTimeout() throws SQLException { return 0; }
  public Logger getParentLogger() throws SQLFeatureNotSupportedException { return null; }
  public <T> T unwrap(Class<T> iface) throws SQLException { return null; }
  public boolean isWrapperFor(Class<?> iface) throws SQLException { return false; }
  public Connection getConnection() throws SQLException { return null; }
  public Connection getConnection(String username, String password) throws SQLException { return null; }
}

    Subclass implementing interface DataSource, inheriting methods from base class:

    package de.scrum_master.app;

import javax.sql.DataSource;

public class SubClass extends BaseClass implements DataSource {}

    Driver application:

    package de.scrum_master.app;

import java.sql.SQLException;

public class Application {
  public static void main(String[] args) throws SQLException {
    System.out.println("Aspect should not kick in");
    new BaseClass().getConnection();
    new BaseClass().getConnection("user", "pw");

    System.out.println("Aspect should kick in");
    new SubClass().getConnection();
    new SubClass().getConnection("user", "pw");
  }
}

    Aspect:

    This aspect uses the pointcut you are currently using.

    package de.scrum_master.aspect;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;

@Aspect
public class DataSourceConnectionAspect {
  @Before("execution(public java.sql.Connection *.getConnection(..)) && target(javax.sql.DataSource)")
  public void myAdvice(JoinPoint thisJoinPoint) {
    System.out.println(thisJoinPoint);
  }
}

    Console log:

    Aspect should not kick in
Aspect should kick in
execution(Connection de.scrum_master.app.BaseClass.getConnection())
execution(Connection de.scrum_master.app.BaseClass.getConnection(String, String))

    No surprises here, everything works as expected. In my opinion this is an efficient way to do it. Of course the aspect code will be woven into each method matching public java.sql.Connection *.getConnection(..)) and there will be a runtime check if target(javax.sql.DataSource) really applies, see also the javap output:

    Compiled from "BaseClass.java"
public class de.scrum_master.app.BaseClass {
  (...)

  public java.sql.Connection getConnection() throws java.sql.SQLException;
    Code:
       0: aload_0
       1: instanceof    #76                 // class javax/sql/DataSource
       4: ifeq          21
       7: invokestatic  #70                 // Method de/scrum_master/aspect/DataSourceConnectionAspect.aspectOf:()Lde/scrum_master/aspect/DataSourceConnectionAspect;
      10: getstatic     #58                 // Field ajc$tjp_0:Lorg/aspectj/lang/JoinPoint$StaticPart;
      13: aload_0
      14: aload_0
      15: invokestatic  #64                 // Method org/aspectj/runtime/reflect/Factory.makeJP:(Lorg/aspectj/lang/JoinPoint$StaticPart;Ljava/lang/Object;Ljava/lang/Object;)Lorg/aspectj/lang/JoinPoint;
      18: invokevirtual #74                 // Method de/scrum_master/aspect/DataSourceConnectionAspect.myAdvice:(Lorg/aspectj/lang/JoinPoint;)V
      21: aconst_null
      22: areturn

  public java.sql.Connection getConnection(java.lang.String, java.lang.String) throws java.sql.SQLException;
    Code:
       0: aload_1
       1: astore        4
       3: aload_2
       4: astore        5
       6: aload_0
       7: instanceof    #76                 // class javax/sql/DataSource
      10: ifeq          31
      13: invokestatic  #70                 // Method de/scrum_master/aspect/DataSourceConnectionAspect.aspectOf:()Lde/scrum_master/aspect/DataSourceConnectionAspect;
      16: getstatic     #79                 // Field ajc$tjp_1:Lorg/aspectj/lang/JoinPoint$StaticPart;
      19: aload_0
      20: aload_0
      21: aload         4
      23: aload         5
      25: invokestatic  #82                 // Method org/aspectj/runtime/reflect/Factory.makeJP:(Lorg/aspectj/lang/JoinPoint$StaticPart;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Lorg/aspectj/lang/JoinPoint;
      28: invokevirtual #74                 // Method de/scrum_master/aspect/DataSourceConnectionAspect.myAdvice:(Lorg/aspectj/lang/JoinPoint;)V
      31: aconst_null
      32: areturn

  (...)
}

    I.e. the runtime check also occurs for classes happening to implement these very special method pattern if the current instance is not a DataSource. But that should be rare.

    There is one alternative involving ITD (inter-type declaration): You can make the base class directly implement the interface and then return to using your more efficient original pointcut. In annotation-based syntax this would like this:

    package de.scrum_master.aspect;

import javax.sql.DataSource;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;
import org.aspectj.lang.annotation.DeclareParents;

@Aspect
public class DataSourceConnectionAspect {
  @DeclareParents("de.scrum_master.app.BaseClass")
  private DataSource dataSource;

  @Before("execution(public java.sql.Connection javax.sql.DataSource+.getConnection(..))")
  public void myAdvice(JoinPoint thisJoinPoint) {
    System.out.println(thisJoinPoint);
  }
}

    Unfortunately, with the AspectJ version I used to test this, the AspectJ compiler throws an exception. That might be a bug, I will look into it later and report to the maintainer. Update: I created AspectJ bug ticket #550494 for this problem. Update 2: The bug was fixed in AspectJ 1.9.5.

    But if you just use native AspectJ syntax it works. The only bad news is that if you use javac + LTW and rely on the AspectJ weaver to finish the aspect during class-loading, this will no longer work. You have to compile an aspect in native syntax with the AspectJ compiler ajc.

    package de.scrum_master.aspect;

import javax.sql.DataSource;

import de.scrum_master.app.BaseClass;

public aspect DataSourceConnectionAspect {
  declare parents: BaseClass implements DataSource;

  before() : execution(public java.sql.Connection javax.sql.DataSource+.getConnection(..)) {
    System.out.println(thisJoinPoint);
  }
}

    Now the console log changes to:

    Aspect should not kick in
execution(Connection de.scrum_master.app.BaseClass.getConnection())
execution(Connection de.scrum_master.app.BaseClass.getConnection(String, String))
Aspect should kick in
execution(Connection de.scrum_master.app.BaseClass.getConnection())
execution(Connection de.scrum_master.app.BaseClass.getConnection(String, String))

    Of course "Aspect should not kick in" no longer applies here because now we do expect it to kick in, of course, as BaseClass now directly implements the DataSource interface.

    A little disclaimer: This approach only works if all the interface methods are really present in the base class, which fortunately is the case for org.apache.tomcat.jdbc.pool.DataSourceProxy, i.e. you can adapt my aspect accordingly. If the base class would only implement part of the expected interface methods, you could also add them via ITD in native syntax, but I am not going to elaborate on that here, my answer is quite long already.

    Last, but not least, this is what the byte code looks like with the new approach:

    Compiled from "BaseClass.java"
public class de.scrum_master.app.BaseClass implements javax.sql.DataSource {
  (...)

  public java.sql.Connection getConnection() throws java.sql.SQLException;
    Code:
       0: getstatic     #58                 // Field ajc$tjp_0:Lorg/aspectj/lang/JoinPoint$StaticPart;
       3: aload_0
       4: aload_0
       5: invokestatic  #64                 // Method org/aspectj/runtime/reflect/Factory.makeJP:(Lorg/aspectj/lang/JoinPoint$StaticPart;Ljava/lang/Object;Ljava/lang/Object;)Lorg/aspectj/lang/JoinPoint;
       8: astore_1
       9: invokestatic  #70                 // Method de/scrum_master/aspect/DataSourceConnectionAspect.aspectOf:()Lde/scrum_master/aspect/DataSourceConnectionAspect;
      12: aload_1
      13: invokevirtual #74                 // Method de/scrum_master/aspect/DataSourceConnectionAspect.ajc$before$de_scrum_master_aspect_DataSourceConnectionAspect$1$19879111:(Lorg/aspectj/lang/JoinPoint;)V
      16: aconst_null
      17: areturn

  public java.sql.Connection getConnection(java.lang.String, java.lang.String) throws java.sql.SQLException;
    Code:
       0: aload_1
       1: astore        4
       3: aload_2
       4: astore        5
       6: getstatic     #77                 // Field ajc$tjp_1:Lorg/aspectj/lang/JoinPoint$StaticPart;
       9: aload_0
      10: aload_0
      11: aload         4
      13: aload         5
      15: invokestatic  #80                 // Method org/aspectj/runtime/reflect/Factory.makeJP:(Lorg/aspectj/lang/JoinPoint$StaticPart;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Lorg/aspectj/lang/JoinPoint;
      18: astore_3
      19: invokestatic  #70                 // Method de/scrum_master/aspect/DataSourceConnectionAspect.aspectOf:()Lde/scrum_master/aspect/DataSourceConnectionAspect;
      22: aload_3
      23: invokevirtual #74                 // Method de/scrum_master/aspect/DataSourceConnectionAspect.ajc$before$de_scrum_master_aspect_DataSourceConnectionAspect$1$19879111:(Lorg/aspectj/lang/JoinPoint;)V
      26: aconst_null
      27: areturn

  (...)
}

    If you compare the two javap logs you will not only notice that now it says implements javax.sql.DataSource but also that in the old version there were 22/32 bytecode instructions for the two methods, whereas the in new version there are just 17/27. For instance, in the old version you see instanceof #76 // class javax/sql/DataSource. In the new version the instanceof check is no longer necessary.

    You can decide by yourself if this makes it worth for you to use ITD and native syntax. I personally use native syntax and ajc anyway, so I would do it. If you never used the AspectJ compiler before and exclusively use LTW, the decision might be different. If there even would be a measurable performance gain is yet another question. I assume that in scenarios involving SQL database calls, probably not AspectJ is what eats up your performance. ;-) I was just curious to find out and answer your question.

    Update: Alternative solution without ITD

    According to your comment you want to avoid ITD, even though I think it is a clean and elegant solution. But there is also a way to optimise pointcut matching and performance like this:

    package de.scrum_master.aspect;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;
import org.aspectj.lang.annotation.Pointcut;

@Aspect
public class AlternativeSolutionAspect {
  @Pointcut("execution(public java.sql.Connection getConnection(..))")
  private static void getConnection() {}

  @Pointcut("within(javax.sql.DataSource+)")
  private static void withinDataSource() {}

  @Pointcut("target(javax.sql.DataSource)")
  private static void targetDataSource() {}

  @Before("withinDataSource() && getConnection()")
  public void interceptStatically(JoinPoint thisJoinPoint) {
    System.out.println("[static] " + thisJoinPoint);
  }

  @Before("!withinDataSource() && getConnection() && targetDataSource()")
  public void interceptDynamically(JoinPoint thisJoinPoint) {
    System.out.println("[dynamic] " + thisJoinPoint);
  }
}

    Explanation:

    Now what does that mean if we compare my DataSourceConnectionAspect to this AlternativeSolutionAspect? First let me add another sample class in order to make it clearer:

    package de.scrum_master.app;

import java.sql.Connection;
import java.sql.SQLException;

import javax.sql.DataSource;

public class SubClassOverridingMethods extends BaseClass implements DataSource {
  @Override
  public Connection getConnection() throws SQLException {
    return super.getConnection();
//    return null;
  }

  @Override
  public Connection getConnection(String username, String password) throws SQLException {
    return super.getConnection(username, password);
//    return null;
  }
}

    Now we extend the driver application by additional method calls:

    package de.scrum_master.app;

import java.sql.SQLException;

public class Application {
  public static void main(String[] args) throws SQLException {
    System.out.println("Aspect should not kick in without ITD, but should with ITD");
    new BaseClass().getConnection();
    new BaseClass().getConnection("user", "pw");

    System.out.println("Aspect should kick in");
    new SubClass().getConnection();
    new SubClass().getConnection("user", "pw");

    System.out.println("Aspect should kick in");
    new SubClassOverridingMethods().getConnection();
    new SubClassOverridingMethods().getConnection("user", "pw");
  }
}

    The rest remains like in my example above.

    Console log for DataSourceConnectionAspect:

    Aspect should not kick in without ITD, but should with ITD
execution(Connection de.scrum_master.app.BaseClass.getConnection())
execution(Connection de.scrum_master.app.BaseClass.getConnection(String, String))
Aspect should kick in
execution(Connection de.scrum_master.app.BaseClass.getConnection())
execution(Connection de.scrum_master.app.BaseClass.getConnection(String, String))
Aspect should kick in
execution(Connection de.scrum_master.app.SubClassOverridingMethods.getConnection())
execution(Connection de.scrum_master.app.BaseClass.getConnection())
execution(Connection de.scrum_master.app.SubClassOverridingMethods.getConnection(String, String))
execution(Connection de.scrum_master.app.BaseClass.getConnection(String, String))

    In case 3 you see 4 lines of log output for 2 method calls because the overriding methods call super.getConnection(..). If they would just do something without using super calls, there would only be one log line per method call, of course.

    Console log for AlternativeSolutionAspect:

    Aspect should not kick in without ITD, but should with ITD
Aspect should kick in
[dynamic] execution(Connection de.scrum_master.app.BaseClass.getConnection())
[dynamic] execution(Connection de.scrum_master.app.BaseClass.getConnection(String, String))
Aspect should kick in
[static] execution(Connection de.scrum_master.app.SubClassOverridingMethods.getConnection())
[dynamic] execution(Connection de.scrum_master.app.BaseClass.getConnection())
[static] execution(Connection de.scrum_master.app.SubClassOverridingMethods.getConnection(String, String))
[dynamic] execution(Connection de.scrum_master.app.BaseClass.getConnection(String, String))

    As we do not use ITD here, nothing gets intercepted for case 1. Case 2 is intercepted dynamically while in case 3 the overriding methods can be determined statically and the super method dynamically. Again, if there were no super calls, we would only have one line of log output per method call for case 3.

    P.S.: Your own solution would also match twice in case of super calls, just in case you wondered. But it would match dynamically both times, making it slower.