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

CaffeineCache does not support LoadingCache consistently #25173

Closed
stefano-salmaso opened this issue Jun 1, 2020 · 5 comments
Closed

CaffeineCache does not support LoadingCache consistently #25173

stefano-salmaso opened this issue Jun 1, 2020 · 5 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@stefano-salmaso
Copy link

Hi all
CaffeineCache implementation override method public ValueWrapper get(Object key) and there is a specific check when a LoadingCache is provided.
In this case, the CacheLoader of the LoadingCache is used to retrieve the data (if not already present int the cache).

I don't see the same behavior for the method public <T> T get(Object key, @Nullable Class<T> type).
In this case, the CacheLoader of the LoadingCache is never used. Using only this method... the cache will be always empty.

Is there any reason for this different behavior?

Thanks
Stefano

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 1, 2020
@snicoll
Copy link
Member

snicoll commented Jun 2, 2020

Yes. The cache abstraction is meant to be used as follows:

@Cacheable
public MyObject computeSomethingExpensive(String key) { ... }

Using LoadingCache is incompatible with this pattern which explains the behaviour of get. If you have more questions, please follow-up on StackOverflow, as mentioned in the guidelines for contributing, we prefer to use the issue tracker only for bugs and enhancements.

@snicoll snicoll closed this as completed Jun 2, 2020
@snicoll snicoll added for: stackoverflow A question that's better suited to stackoverflow.com and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 2, 2020
@stefano-salmaso
Copy link
Author

Hi @snicoll , I agree with you that the use of a loading cache with @cacheble is incompatible (and I would add "strange").
Sorry if I haven't used Stackoverflow ... but I would take advantage of this already created issue to ask you why the loading cache is used in the public ValueWrapper get (Object key) method to retrieve the value.
If mean... if LoadingCache is incompatible, it shouldn't be used in public ValueWrapper get (Object key) also.

@snicoll
Copy link
Member

snicoll commented Jun 2, 2020

I overlooked your report, sorry about that. I find it strange that we have a case for supporting the loading cache there so let's reopen to at least check.

You haven't really shared why you're reporting this. What made you create this issue in the first place?

@snicoll snicoll reopened this Jun 2, 2020
@snicoll snicoll added status: waiting-for-triage An issue we've not yet triaged or decided on and removed for: stackoverflow A question that's better suited to stackoverflow.com labels Jun 2, 2020
@stefano-salmaso
Copy link
Author

stefano-salmaso commented Jun 2, 2020

I have an application where we are using the Spring Cache abstraction without the usage of @Cacheable. In my specific case, having a LoadingCache is useful, but I discovered that it doesn't work with the method public <T> T get(Object key, @Nullable Class<T> type).

Checking the source code of CaffeineCache I found this different behavior.
Again, I agree with you that @Cacheable and LoadingCache are incompatible but I think we should be coherent in the CaffeineCache.
(In this case, removing the usage of the Loadingcache could be the right choice)

@snicoll
Copy link
Member

snicoll commented Jun 2, 2020

Thanks for the feedback and that clarifies the issue quite a bit. If we can consistently call the LoadingCache, I'd rather do that as removing the usage is a backward incompatible change and I am not keen to do that.

stefano-salmaso pushed a commit to stefano-salmaso/spring-framework that referenced this issue Jun 5, 2020
When CaffeineCache is created with a LoadingCache, the LoadingCache is
used with the method `public ValueWrapper get(Object key)` but not
with method `public <T> T get(Object key, @nullable Class<T> type)`.
Using LoadingCache is incompatible with @Cacheable pattern but
consistently call the LoadingCache in both methods is better than now.
Removing the usage is a backward-incompatible change.

Closes spring-projectsgh-25173
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 24, 2020
@snicoll snicoll self-assigned this Jul 24, 2020
@snicoll snicoll added this to the 5.3 M2 milestone Jul 24, 2020
@snicoll snicoll changed the title CaffeineCache with LoadingCache CaffeineCache does not support LoadingCache consistently Jul 24, 2020
@snicoll snicoll closed this as completed in f3cedf2 Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants