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
-
33216
-
35189
-
35556
Comment From: fibsifan
I was able to check Spring versions 6.2.10 and 6.2.11.
Comment From: 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.
Comment From: sbrannen
In case someone else gets a chance to investigate this further before I do, I have pushed related tests to the following branch.
https://github.com/spring-projects/spring-framework/compare/main...sbrannen:spring-framework:issues/gh-35667-cache-key-generation-from-static-methods
Comment From: 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.
Comment From: 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.
Comment From: 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.
Comment From: 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 ba6166a02d8bc2a7a9e1b57c7431fee4ca3854b1 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 a6f6ecfe6c2d2f20b23c3d725614bc36c9c0e8f4 and 717358b56bce7d1942a0c5ae3ba45cf5e967c7d9.
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
staticmethod, since astaticmethod 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.
Comment From: github-actions[bot]
Fixed via b1f5b61bcd79fca7e697ec9ea263125cd7e7b557