`getPubliclyAccessibleMethodIfPossible()` returns hidden static method

[fibsifan]

Overview

When static methods are used for cache key creation and 2 static methods have the same signature in a class inheritance hierarchy, then only one static method is used for cache key generation. I created a small example project: https://github.com/fibsifan/spring-cache-error

The most relevant part is in the Controller:

    @Cacheable(cacheNames = "cachedController", key = "T(de.jball.spring.caching.keys.ParentCacheKeyGenerator).getCacheKey()")
    @GetMapping("parent")
    public String getResponseWithParentCacheKey() {
        return "ResponseWithParentCacheKey";
    }

    @Cacheable(cacheNames = "cachedController", key = "T(de.jball.spring.caching.keys.ChildCacheKeyGenerator).getCacheKey()")
    @GetMapping("child")
    public String getResponseWithChildCacheKey() {
        return "ResponseWithChildCacheKey";
    }

If GET http://localhost:8080/parent is called first, then calls to /child always return ResponseWithParentCacheKey and vice versa.

Related Issues

• Invoke init/destroy/SpEL methods via public declaring type whenever possible #33216
• Restore preference for interface (most abstract) method in getPubliclyAccessibleMethodIfPossible #35189
• SpEL expression parser uses more CPU after upgrade to 6.2.9 #35556

[fibsifan]

I was able to check Spring versions 6.2.10 and 6.2.11.

[sbrannen]

Hi @fibsifan,

Congratulations on submitting your first issue for the Spring Framework! 👍

I verified that your sample application demonstrates the behavior you have described; however, I have yet to reproduce it in local integration tests without Spring Boot and Tomcat involved. So, we will investigate further and get back to you.

[sbrannen]

In case someone else gets a chance to investigate this further before I do, I have pushed related tests to the following branch.

main...sbrannen:spring-framework:issues/gh-35667-cache-key-generation-from-static-methods

[fibsifan]

In the debugger I found, that the difference in behaviour between my example and the working tests seems to be in org.springframework.util.ClassUtils#getPubliclyAccessibleMethodIfPossible:

With my example, this method returns the method of the parent class. Within the tests, this method correctly returns the method itself. I can see, that the method recently underwent some changes, but I don't recognize any logic specific to static methods, so I'm not sure whether the newer version works based on correct reasons.

[fibsifan]

I rebased the tests to Spring 6.2.11 and they're failing. With 6.2.12 they're green.

I'm still not sure, whether the tests pass for the right reason. The change in ClassUtils seems related to the Module System with which I'm unfamiliar. Maybe a different example could be constructed with Parent and Child being in different modules, where the parent still would replace the child method?

But as said I'm unfamiliar with doing so, for now I'll request a Spring dependency Update with the vendor that delivers their product with spring 6.2.11.

[sbrannen]

@fibsifan, thanks for debugging this on your own and providing feedback. Much appreciated. 👍

The changes to getPubliclyAccessibleMethodIfPossible(Method, Class<?>) may well have caused this regression, which appears to have been fixed in 6.2.12 in conjunction with #35556.

However, we actually should not be attempting to get a "publicly accessible method" from further up the type hierarchy for a static method, since a static method can never be overridden in the first place. So, I'll take a closer look at this.

[sbrannen]

The changes to getPubliclyAccessibleMethodIfPossible(Method, Class<?>) may well have caused this regression, which appears to have been fixed in 6.2.12 in conjunction with #35556.

I have confirmed that's the case.

Commit ba6166a changed the logic to find the "highest publicly accessible method in the supplied method's type hierarchy."

Consequently, when searching for a publicly accessible equivalent method for getCacheKey() in ChildCacheKeyGenerator, the algorithm continued searching the type hierarchy and found getCacheKey() in ParentCacheKeyGenerator.

As I mentioned above, that was fixed in 6.2.12 in commits a6f6ecf and 717358b.

I'm still not sure, whether the tests pass for the right reason. The change in ClassUtils seems related to the Module System with which I'm unfamiliar. Maybe a different example could be constructed with Parent and Child being in different modules, where the parent still would replace the child method?

That's indeed the case. See below.

However, we actually should not be attempting to get a "publicly accessible method" from further up the type hierarchy for a static method, since a static method can never be overridden in the first place. So, I'll take a closer look at this.

My assumption was correct. If you remove the public declaration for ChildCacheKeyGenerator, the algorithm still incorrectly considers getCacheKey() in ParentCacheKeyGenerator to be a publicly accessible equivalent method for getCacheKey() in ChildCacheKeyGenerator.

Aborting the search algorithm for static methods fixes the issue.

In light of that, I have labeled this a bug, and I'll fix it in 6.2.13.