Skip to content

Conversation

@shakuzen
Copy link
Member

I came across this while trying out the new Caffeine caching auto-configuration. After reading the Spring Boot documentation on Caffeine caching, I was surprised that my provided CacheLoader<String, List<String>> bean was not being used by the auto-configuration. If this is the intended behavior, then this pull request can be closed, but I think the documentation could be a bit more clear.

The UT added here fails without the main code changes. Similar to the UT, I wanted to use the Caffeine auto-configuration but provide a CacheLoader to be used, like the following in a @Configuration class:

@Bean
public CacheLoader<String, List<String>> cacheLoader(SlowService service) {
    return service::getNames;
}

When using the Caffeine cache auto-configuration, the prior behavior required that the user provided `CacheLoader` be specifically of type `CacheLoader<Object, Object>`. Otherwise it would not be used as part of the auto-configuration. This required using `Object` types and casting in the `CacheLoader` logic. With this commit, a specifically-typed `CacheLoader` bean can be provided and is used by the auto-configuration.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 28, 2016
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 28, 2016
@philwebb philwebb added this to the 1.4.1 milestone Aug 28, 2016
@philwebb
Copy link
Member

Seems like we should support different generics. I wonder if CacheLoader<?, ?> would work rather than dropping the generics all together?

@ben-manes
Copy link

Can we apply any fixes here to Guava's as well? I would expect similar problems.

@philwebb
Copy link
Member

@ben-manes Thanks for the prompt, I've opened #6778 to tackle that.

@snicoll
Copy link
Member

snicoll commented Aug 29, 2016

For the record, yes this is the intended behaviour. The setter in the related CacheManager is public void setCacheLoader(CacheLoader<Object, Object> cacheLoader) to clearly indicate that we won't support typed caches at that level.

CaffeineCache is using internally a Cache<Object, Object> (see the return type of getNativeCache). I think it's confusing to have a CacheLoader<String, List<String>> in the context and expect Spring Boot to pick that up and associate it with the cache manager. The reason for that is that this CacheLoader is used as a loader for all caches that are managed by that instance. For the reason I explained above, caches in spring land are untyped so the only way for this to make sense is if you would created the cache infrastructure yourself, and not via auto-config.

I am -1 to apply this change without a corresponding change in Spring Framework. The fact you dropped the generic type is a smell.

@snicoll snicoll removed the type: bug A general bug label Aug 29, 2016
@snicoll snicoll added the type: documentation A documentation update label Aug 31, 2016
@snicoll snicoll closed this in f93775e Aug 31, 2016
@snicoll
Copy link
Member

snicoll commented Aug 31, 2016

As we've discussed on gitter I've improved the documentation to make it clear that only CacheLoader<Object, Object> is taken care by the auto-configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: documentation A documentation update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants