Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Caching Abstraction ignores method name? [SPR-8933] #13573

Closed
spring-issuemaster opened this issue Dec 15, 2011 · 11 comments
Closed

Caching Abstraction ignores method name? [SPR-8933] #13573

spring-issuemaster opened this issue Dec 15, 2011 · 11 comments
Assignees

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Dec 15, 2011

Alexander Derenbach opened SPR-8933 and commented

When I read the reference documentation I thought the caching abstraction is on method level, but it seems it is only on the parameter value of the methods.

If I have several methods with the same parameter type it get always the same return value:

@Component
public class CacheTestImpl implements CacheTest {
@Cacheable("databaseCache")
public Long test1() {
return 1L;
}

@Cacheable("databaseCache")
public Long test2() {
    return 2L;
}

@Cacheable("databaseCache")
public Long test3() {
    return 3L;
}

   
@Cacheable("databaseCache")
public String test4() {
    return "4";
}

}

Calling now:

System.out.println(test.test1());
System.out.println(test.test2());
System.out.println(test.test3());
System.out.println(test.test4());

results in:

1
1
1
ClassCastException: java.lang.Long cannot be cast to java.lang.String

Is this the desired behaviour? I would expect:

1
2
3
4

If I use different caches it works.

I can't access github from my place (firewall) so I have added a tar with a small maven project showing this problem.

Greets Alex


Affects: 3.1 GA

Attachments:

Issue Links:

  • #16358 Cacheable javadoc wrongly explains how cache keys are generated

5 votes, 9 watchers

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Dec 15, 2011

Alexander Derenbach commented

There is a possiblity to use only the method name:

@Cacheable(value="databaseCache", key="#root.methodName")
public Long test(Long test) {
return test;
}

But then the parameter value is ignored.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Dec 16, 2011

Alexander Derenbach commented

I found a solution (workaround) for my problem:

@Cacheable(value="databaseCache", key="{#root.methodName,#test}")
public Long test(Long test) {
return test;
}

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Dec 16, 2011

Alexander Derenbach commented

If this is the intended behaviour it would be nice if either the value field of @Cacheable has a default, so that it is possible to add a annotation like this:

@Target({ElementType.METHOD, ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
@Cacheable(key="{#root.methodName,#root.args}")
public @interface MethodCacheable {

}

...

@MethodCacheable("cache")
public Long test(Lont t1) {
}

or to add a new annotation for that to Spring itself

Greets Alex

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Dec 16, 2011

Alexander Derenbach commented

I was wrong

@Cacheable(key="{#root.methodName,#root.args}") does not work because the args array is always a new one...

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Dec 22, 2011

Costin Leau commented

This is not a bug. All 4 methods use the same key and the same cache, thus, after you call test1, all the other methods will automatically find in the cache the same value.
If you want to differentiate per method, you just need to add the method name as a key into the cache.
This distinction does not happen by default since for the most part, different methods can share the same result.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 22, 2012

Ned Mules commented

Hi Costin

Thanks for your work on this great feature.

But I agree with Alex that its behaviour is counter-intuitive.

You said:
"This distinction does not happen by default since for the most part, different methods can share the same result."

I may be missing something but why would you typically want methods with different names and the same parameters to return the same values from the cache?

For example, if I was building a music catalogue I would like to do this:

@Cacheable("musicCache")
public Artist getArtist(int artistId)

@Cacheable("musicCache")
public List<Album> getAlbums(int artistId)

As far as I can see, for this to work I would have to define different caches (with accompanying boilerplate ehcache XML configuration and associated maintenance overhead) for Artists, AlbumLists, etc, etc, and remember to use the correct one with the correct method. Or as you said, I would have to explicitly specify the method name as a key wherever I use @Cacheable. It's not clear how to do this - Alex said the examples above didn't work.

My solution to this was to write a "MethodAwareCacheKeyGenerator" which implements KeyGenerator and is manually wired into a CacheInterceptor in my @Configuration class. This works but it's about 40 lines of code that I think should be unnecessary.

Could you please reconsider changing this behaviour or at least explicitly documenting the default behaviour and how to make it cache on method name?

At the moment, the documentation says "Out of the box, the caching abstraction uses a simple hash-code based KeyGenerator that computes the key based on the hashes of all objects used for method invocation".

My assumption was that the method name was one of the "objects" used. It would help if the documentation stated that only the arguments are used by default, and not the method name.

Thanks
Ned

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 23, 2012

Alexander Derenbach commented

Hello Ned,

I am glad that I am not the only one who miss understood it. (or expected something else).
My solution for my problem is now this (as mentioned above)

@Cacheable(value="databaseCache", key="{#root.methodName,#test}")
public Long test(Long test) { return test; }

this works fine.

But I agree with you that at least the documentation could perhaps mention the behavior a bit better, since they are talking about "method caching" which leads me to my expecting of the cache.

Greets Alex

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 8, 2013

Mukarram Baig commented

Hello Costin and gang - Thanks for the great work thus far! I too tend to agree with Ned that the behavior is slightly counter-intuitive. I was trying to cache calls to a third party JAR via the cache:advice route and Ned's suggestion to have a MethodAwareCacheKeyGenerator helped me out. Can we provide such a key generator from Spring rather than having each user hand-roll their own? I am attaching an implementation that I am using.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 21, 2013

Mukarram Baig commented

Updated definition

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 7, 2013

Alex Wajda commented

Please do not close it as "Invalid" because it is pretty much valid.
The current behaviour is too counter intuitive.
The following conclusion is at least illogical.

...for the most part, different methods can share the same result
The different methods are aimed to have different semantics and event though the input can be the same the result will most likely to be different. Otherwise why would 2 methods with identical semantics co-exist?

I would vote for making the (methodName,arg1,arg2,...,argN) to be the default key. So that even the key spaces for overloaded methods would not overlap. To me that is the most sensible default.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 1, 2014

chris marx commented

I agree, I think most people would assume that the method signature would be part of the cache key generation-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.