Search code examples
spring-aopstatic-analysisfindbugspmdspring-cache

Static Analysis tool to catch self-invocation bypassing Spring cache @Cacheable method


I understand that this is because of the way proxies are created for handling caching, transaction related functionality in Spring. And the way to fix it is use AspectJ but I donot want to take that route cause it has its own problems. Can I detect self-invocation using any static analyis tools?

@Cacheable(value = "defaultCache", key = "#id")
public Person findPerson(int id) {
   return getSession().getPerson(id);
} 

public List<Person> findPersons(int[] ids) {
   List<Person> list = new ArrayList<Person>();
       for (int id : ids) {
      list.add(findPerson(id));
    }
   return list;
} 

Solution

  • If it would be sufficient for you to detect internal calls, you could use native AspectJ instead of Spring AOP for that and then throw runtime exceptions or log warnings every time this happens. That is not static analysis, but better than nothing. On the other hand, if you use native AspectJ, you are not limited to Spring proxies anyway and the aspects would work for self-invocation too.

    Anyway, here is what an aspect would look like, including an MCVE showing how it works. I did it outside of Spring, which is why I am using a surrogate @Component annotation for demo purposes.

    Update: Sorry for targeting @Component classes instead of @Cacheable classes/methods, but basically the same general approach I am showing here would work in your specific case, too, if you simply adjust the pointcut a bit.

    Component annotation:

    package de.scrum_master.app;
    
    import static java.lang.annotation.ElementType.TYPE;
    import static java.lang.annotation.RetentionPolicy.RUNTIME;
    
    import java.lang.annotation.Retention;
    import java.lang.annotation.Target;
    
    @Retention(RUNTIME)
    @Target(TYPE)
    public @interface Component {}
    

    Sample classes (components and non-components):

    This component is to be called by other components should not lead to exceptions/warnings:

    package de.scrum_master.app;
    
    @Component
    public class AnotherComponent {
      public void doSomething() {
        System.out.println("Doing something in another component");
      }
    }
    

    This class is not a @Component, so the aspect should ignore self-invocation inside it:

    package de.scrum_master.app;
    
    public class NotAComponent {
      public void doSomething() {
        System.out.println("Doing something in non-component");
        new AnotherComponent().doSomething();
        internallyCalled("foo");
      }
    
      public int internallyCalled(String text ) {
        return 11;
      }
    }
    

    This class is a @Component. The aspect should flag internallyCalled("foo"), but not new AnotherComponent().doSomething().

    package de.scrum_master.app;
    
    @Component
    public class AComponent {
      public void doSomething() {
        System.out.println("Doing something in component");
        new AnotherComponent().doSomething();
        internallyCalled("foo");
      }
    
      public int internallyCalled(String text ) {
        return 11;
      }
    }
    

    Driver application:

    Please note that I am creating component instances throughout this sample code with new instead of requesting beans from the application context, like I would do in Spring. But you can ignore that, it is just an example.

    package de.scrum_master.app;
    
    public class Application {
      public static void main(String[] args) {
        new NotAComponent().doSomething();
        new AComponent().doSomething();
      }
    }
    

    Console log when running without aspect:

    Doing something in non-component
    Doing something in another component
    Doing something in component
    Doing something in another component
    

    Now with the aspect, instead of the last message we would expect an exception or a logged warning. Here is how to do that:

    Aspect:

    Sorry for using native AspectJ syntax here. Of course, you could also use annotation-based syntax.

    package de.scrum_master.aspect;
    
    import de.scrum_master.app.*;
    
    public aspect SelfInvocationInterceptor {
      Object around(Object caller, Object callee) :
        @within(Component) &&
        call(* (@Component *).*(..)) &&
        this(caller) &&
        target(callee)
      {
        if (caller == callee)
          throw new RuntimeException(
            "Self-invocation in component detected from "  + thisEnclosingJoinPointStaticPart.getSignature() +
            " to "+ thisJoinPointStaticPart.getSignature()
          );
        return proceed(caller, callee);
      }
    }
    

    Console log when running with aspect:

    Doing something in non-component
    Doing something in another component
    Doing something in component
    Doing something in another component
    Exception in thread "main" java.lang.RuntimeException: Self-invocation in component detected from void de.scrum_master.app.AComponent.doSomething() to int de.scrum_master.app.AComponent.internallyCalled(String)
        at de.scrum_master.app.AComponent.internallyCalled_aroundBody3$advice(AComponent.java:8)
        at de.scrum_master.app.AComponent.doSomething(AComponent.java:8)
        at de.scrum_master.app.Application.main(Application.java:6)
    

    I think, you can use this solution and maybe rather log warnings instead of throwing exceptions in order to softly guide your co-workers to inspect and improve their AOP-dependent Spring components. Sometimes maybe they do not wish self-invocation to trigger an aspect anyway, it depends on the situation. You could run the Spring application in full AspectJ mode and then, after evaluating the logs, switch back to Spring AOP. But maybe it would be simpler to just use native AspectJ to begin with and avoid the self-invocation problem altogether.


    Update: In AspectJ you can also make the compiler throw warnings or errors if certain conditions are met. In this case you could only statically determine calls from components to other components, but without differentiating between self-invocation and calls on other methods from other components. So this does not help you here.

    Please also notice that this solution is limited to classes annotated by @Component. If your Spring bean is instantiated in other ways, e.g. via XML configuration or @Bean factory method, this simple aspect does not work. But it could easily be extended by checking if the intercepted class is a proxy instance and only then decide to flag self-invocations. Then unfortunately, you would have to weave the aspect code into all of your application classes because the check can only happen during runtime.

    I could explain many more things, such as using self-injection and call internal methods on the injected proxy instance instead of via this.internallyCalled(..). Then the self-invocation problem would be solved too and this approach also works in Spring AOP.