MemcachedAutoConfiguration should not be conditional on the absence of other cachemanagers
davidkarlsen opened this issue · 6 comments
I suggest the following patch:
git diff memcached-spring-boot-autoconfigure/src/main/java/io/sixhours/memcached/cache/MemcachedCacheAutoConfiguration.java
diff --git a/memcached-spring-boot-autoconfigure/src/main/java/io/sixhours/memcached/cache/MemcachedCacheAutoConfiguration.java b/memcached-spring-boot-autoconfigure/src/main/java/io/sixhours/memcached/cache/MemcachedCacheAutoConfiguration.java
index 4bd73b1..6ae1ab1 100644
--- a/memcached-spring-boot-autoconfigure/src/main/java/io/sixhours/memcached/cache/MemcachedCacheAutoConfiguration.java
+++ b/memcached-spring-boot-autoconfigure/src/main/java/io/sixhours/memcached/cache/MemcachedCacheAutoConfiguration.java
@@ -38,7 +38,7 @@ import org.springframework.context.annotation.Import;
@Configuration
@ConditionalOnMissingSpringCacheType
@ConditionalOnBean(CacheAspectSupport.class)
-@ConditionalOnMissingBean({CacheManager.class, CacheResolver.class})
+//@ConditionalOnMissingBean({CacheManager.class, CacheResolver.class})
@EnableConfigurationProperties(MemcachedCacheProperties.class)
@AutoConfigureBefore(CacheAutoConfiguration.class)
@AutoConfigureAfter(name = "org.springframework.cloud.autoconfigure.RefreshAutoConfiguration")
The user will likely want memcached even if other cachemanagers or resolvers should exist. This would also be in line with previous behaviour.
@davidkarlsen Thank you for suggestion.
Although I do see the value in removing this condition, I am inclined to keep the conditions as they are. As a side note, this is actually how the auto-configuration was working from the first release of the library.
We have tried to follow the same conditions applied with Spring's CacheAutoConfiguration for auto-of-the-box supported cache types. The reason behind this is to allow painless and easy switch between Spring's cache types to memcached, and vice versa. No matter of the configuration a user might be using in their Spring Boot Project, the logic behind cache auto-configuration would remain the same. One case, on top of my head, where this could break user's expected behavior is switching from Spring cache type to memcached when the user is using a different CacheManager bean per Spring profile. There are most probably other cases this could happen.
Hope this clarifies a bit the reason behind including this condition, and why I would prefer to keep it the same.
This was a brain-fart on my side - it works - and it works like before. :)
@igorbolic BTW - release ready soon?
@davidkarlsen Yes, releasing today. Just some more things left (dependencies & Gradle version upgrades). I'll post the info here when it's done
@davidkarlsen The version 2.0.0 has been released on JCenter & Maven Central
yay!