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

Improve documentation of spring.cache.type=none #33694

Closed
micheljung opened this issue Jan 5, 2023 · 5 comments
Closed

Improve documentation of spring.cache.type=none #33694

micheljung opened this issue Jan 5, 2023 · 5 comments
Assignees
Labels
status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-feedback We need additional information before we can continue type: documentation A documentation update
Milestone

Comments

@micheljung
Copy link

micheljung commented Jan 5, 2023

Summary

The doc can be interpreted such that spring.cache.type=none disables caching, when in fact it's not really what it does. The documentation or behavior of Spring Boot should be improved in this regard.

Details

If you want to disable caching, you'll quickly find this in the documentation:

When @EnableCaching is present in your configuration, a suitable cache configuration is expected as well. If you need to disable caching altogether in certain environments, force the cache type to none to use a no-op implementation, as shown in the following example:

spring.cache.type=none

So you do that and think your caching is disabled but if you have an explicit CacheManager configured, it is not. This is because all cache configs are @Conditional({ CacheCondition.class }) and @ConditionalOnMissingBean(CacheManager.class).

Therefore, if you specify spring.cache.type=none, all that happens is that NoOpCacheConfiguration will be loaded if there is no CacheManager bean.

This is partially explained in the docs:

If you have not defined a bean of type CacheManager or a CacheResolver named cacheResolver (see CachingConfigurer), Spring Boot tries to detect the following providers (in the indicated order):

  1. Generic
  2. JCache (JSR-107) (EhCache 3, Hazelcast, Infinispan, and others)
  3. Hazelcast
  4. Infinispan
  5. Couchbase
  6. Redis
  7. Caffeine
  8. Cache2k
  9. Simple

In this list, None is missing, even though NoOpCacheConfiguration is also considered, further contributing to the confusion.

I think that:

  1. The doc should make very clear that none only applies when the CacheManager is auto-configured, not if it's explicitly specified.
  2. Or, none should disable caching even if there is an explicit CacheManager
  3. Or, Spring Boot should throw an error when an explicit CacheManager is specified and spring.cache.type is none
  4. None should also be listed
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 5, 2023
@philwebb philwebb added type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 11, 2023
@philwebb philwebb added this to the 2.7.x milestone Jan 11, 2023
@pernelkanic
Copy link

Can I take up this issue?

@wilkinsona
Copy link
Member

Thanks for your interest in contributing but we generally find documentation updates are better handled by the core team.

I'm not sure if we have any at the moment, but please keep an eye out for unassigned issues labelled with ideal for contribution or, if you haven't contributed before, first-timers only.

@snicoll
Copy link
Member

snicoll commented Aug 3, 2023

The doc can be interpreted such that spring.cache.type=none disables caching, when in fact it's not really what it does. The documentation or behavior of Spring Boot should be improved in this regard

I am more than happy to improve the doc but I fail to see what the exact problem this causes. None configures a no-op cache manager that effectively has the same effect as if the cache was not used at all. The advantage is that it still triggers an error if you're trying to use a cache that doesn't exist (and should).

Therefore, if you specify spring.cache.type=none, all that happens is that NoOpCacheConfiguration will be loaded if there is no CacheManager bean.

This is the exact same behavior for anything auto-configuration related. If you're taking control over things, we don't override your decisions. I am not sure I am following the argument. If you set spring.cache.type=redis, it won't provide a redis-based cache manager either if your app defines a CacheManager already What is the difference?

You're also turning your setup into a general problem that it isn't. If you have @EnableCaching on your @SpringBootApplication, and your CacheManager configuration in a separate configuration class, as you should, then your configuration is not taken into account and this makes sure we don't attempt to auto-configurate another cache manager for your slice tests.

I disagree with 1, 2, and 3 for the reason above. 4 is indeed an oversight.

Thoughts?

@pernelkanic hopefully the above should explain further what Andy stated above.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Aug 3, 2023
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Aug 10, 2023
@snicoll snicoll changed the title Improve documentation and/or implementation of spring.cache.type=none Improve documentation of spring.cache.type=none Aug 14, 2023
@snicoll snicoll self-assigned this Aug 14, 2023
@snicoll
Copy link
Member

snicoll commented Aug 14, 2023

In this list, None is missing, even though NoOpCacheConfiguration is also considered, further contributing to the confusion.

No it isn't. Technically it is part of the CacheType enum but only for the purpose of binding and auto-completion. SIMPLE that is before it will always match if we go up to that point as it doesn't have any condition. So 4 isn't an oversight, after all. That said, I've reviewed the sections based on your feedback and rephrased things to be more explicit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-feedback We need additional information before we can continue type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

6 participants