Skip to content

Commit 5aefcc8

Browse files
snicollsnicoll
committed
Prevent early bean initialization with @EnableCaching
Prior to this commmit, any configuration class holding a CacheManager bean would be eagerly instantiated. This is because the CacheConfiguration infrastructure requests all beans of type CacheManager. This commit defers the resolution of the CacheManager as late as possible. Issue: SPR-12336
1 parent 8e5c77d commit 5aefcc8

File tree

5 files changed

+64
-67
lines changed

5 files changed

+64
-67
lines changed

‎spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java

+28-13
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.springframework.beans.BeanUtils;
2222
import org.springframework.beans.factory.BeanFactoryUtils;
2323
import org.springframework.beans.factory.InitializingBean;
24+
import org.springframework.beans.factory.SmartInitializingSingleton;
2425
import org.springframework.cache.CacheManager;
2526
import org.springframework.cache.interceptor.CacheResolver;
2627
import org.springframework.cache.interceptor.KeyGenerator;
@@ -39,7 +40,7 @@
3940
* @since 4.1
4041
*/
4142
public class DefaultJCacheOperationSource extends AnnotationJCacheOperationSource
42-
implements InitializingBean, ApplicationContextAware {
43+
implements InitializingBean, SmartInitializingSingleton, ApplicationContextAware {
4344

4445
private CacheManager cacheManager;
4546

@@ -83,7 +84,7 @@ public void setCacheResolver(CacheResolver cacheResolver) {
8384
}
8485

8586
public CacheResolver getCacheResolver() {
86-
return this.cacheResolver;
87+
return getDefaultCacheResolver();
8788
}
8889

8990
/**
@@ -95,7 +96,7 @@ public void setExceptionCacheResolver(CacheResolver exceptionCacheResolver) {
9596
}
9697

9798
public CacheResolver getExceptionCacheResolver() {
98-
return this.exceptionCacheResolver;
99+
return getDefaultExceptionCacheResolver();
99100
}
100101

101102
@Override
@@ -105,17 +106,13 @@ public void setApplicationContext(ApplicationContext applicationContext) {
105106

106107
@Override
107108
public void afterPropertiesSet() {
108-
Assert.state((this.cacheResolver != null && this.exceptionCacheResolver != null)
109-
|| this.cacheManager != null, "'cacheManager' is required if cache resolvers are not set.");
110-
Assert.state(this.applicationContext != null, "The application context was not injected as it should.");
111-
112109
this.adaptedKeyGenerator = new KeyGeneratorAdapter(this, this.keyGenerator);
113-
if (this.cacheResolver == null) {
114-
this.cacheResolver = new SimpleCacheResolver(this.cacheManager);
115-
}
116-
if (this.exceptionCacheResolver == null) {
117-
this.exceptionCacheResolver = new SimpleExceptionCacheResolver(this.cacheManager);
118-
}
110+
}
111+
112+
@Override
113+
public void afterSingletonsInstantiated() { // Make sure those are initialized on startup
114+
Assert.notNull(getDefaultCacheResolver(), "Cache resolver should have been initialized.");
115+
Assert.notNull(getDefaultExceptionCacheResolver(), "Exception cache resolver should have been initialized.");
119116
}
120117

121118
@Override
@@ -131,11 +128,17 @@ protected <T> T getBean(Class<T> type) {
131128

132129
@Override
133130
protected CacheResolver getDefaultCacheResolver() {
131+
if (this.cacheResolver == null) {
132+
this.cacheResolver = new SimpleCacheResolver(getCacheManager());
133+
}
134134
return this.cacheResolver;
135135
}
136136

137137
@Override
138138
protected CacheResolver getDefaultExceptionCacheResolver() {
139+
if (this.exceptionCacheResolver == null) {
140+
this.exceptionCacheResolver = new SimpleExceptionCacheResolver(getCacheManager());
141+
}
139142
return this.exceptionCacheResolver;
140143
}
141144

@@ -144,4 +147,16 @@ protected KeyGenerator getDefaultKeyGenerator() {
144147
return this.adaptedKeyGenerator;
145148
}
146149

150+
private CacheManager getCacheManager() {
151+
if (this.cacheManager == null) {
152+
this.cacheManager = this.applicationContext.getBean(CacheManager.class);
153+
if (this.cacheManager == null) {
154+
throw new IllegalStateException("No bean of type CacheManager could be found. " +
155+
"Register a CacheManager bean or remove the @EnableCaching annotation " +
156+
"from your configuration.");
157+
}
158+
}
159+
return this.cacheManager;
160+
}
161+
147162
}

‎spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheInterceptorTests.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import org.junit.Test;
2222

23+
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
2324
import org.springframework.cache.CacheManager;
2425
import org.springframework.cache.interceptor.CacheOperationInvoker;
2526
import org.springframework.cache.interceptor.CacheResolver;
@@ -80,8 +81,7 @@ cacheManager, new NamedCacheResolver(cacheManager), // Returns empty list
8081

8182
@Test
8283
public void cacheManagerMandatoryIfCacheResolverNotSetSet() {
83-
thrown.expect(IllegalStateException.class);
84-
thrown.expectMessage("'cacheManager' is required");
84+
thrown.expect(NoSuchBeanDefinitionException.class);
8585
createOperationSource(null, null, null, defaultKeyGenerator);
8686
}
8787

@@ -117,6 +117,7 @@ protected JCacheOperationSource createOperationSource(CacheManager cacheManager,
117117
source.setExceptionCacheResolver(exceptionCacheResolver);
118118
source.setKeyGenerator(keyGenerator);
119119
source.afterPropertiesSet();
120+
source.afterSingletonsInstantiated();
120121
return source;
121122
}
122123

‎spring-context/src/main/java/org/springframework/cache/annotation/AbstractCachingConfiguration.java

+12-45
Original file line numberDiff line numberDiff line change
@@ -53,61 +53,28 @@ public abstract class AbstractCachingConfiguration<C extends CachingConfigurer>
5353

5454
protected CacheErrorHandler errorHandler;
5555

56-
@Autowired(required=false)
57-
private Collection<CacheManager> cacheManagerBeans;
58-
59-
@Autowired(required=false)
60-
private Collection<C> cachingConfigurers;
61-
62-
6356
@Override
6457
public void setImportMetadata(AnnotationMetadata importMetadata) {
6558
this.enableCaching = AnnotationAttributes.fromMap(
6659
importMetadata.getAnnotationAttributes(EnableCaching.class.getName(), false));
6760
Assert.notNull(this.enableCaching,
6861
"@EnableCaching is not present on importing class " +
69-
importMetadata.getClassName());
62+
importMetadata.getClassName());
7063
}
7164

72-
73-
/**
74-
* Determine which {@code CacheManager} bean to use. Prefer the result of
75-
* {@link CachingConfigurer#cacheManager()} over any by-type matching. If none, fall
76-
* back to by-type matching on {@code CacheManager}.
77-
* @throws IllegalArgumentException if no CacheManager can be found; if more than one
78-
* CachingConfigurer implementation exists; if multiple CacheManager beans and no
79-
* CachingConfigurer exists to disambiguate.
80-
*/
81-
@PostConstruct
82-
protected void reconcileCacheManager() {
83-
if (!CollectionUtils.isEmpty(cachingConfigurers)) {
84-
int nConfigurers = cachingConfigurers.size();
85-
if (nConfigurers > 1) {
86-
throw new IllegalStateException(nConfigurers + " implementations of " +
87-
"CachingConfigurer were found when only 1 was expected. " +
88-
"Refactor the configuration such that CachingConfigurer is " +
89-
"implemented only once or not at all.");
90-
}
91-
C cachingConfigurer = cachingConfigurers.iterator().next();
92-
useCachingConfigurer(cachingConfigurer);
93-
}
94-
if (this.cacheManager == null && !CollectionUtils.isEmpty(cacheManagerBeans)) {
95-
int nManagers = cacheManagerBeans.size();
96-
if (nManagers > 1) {
97-
throw new IllegalStateException(nManagers + " beans of type CacheManager " +
98-
"were found when only 1 was expected. Remove all but one of the " +
99-
"CacheManager bean definitions, or implement CachingConfigurer " +
100-
"to make explicit which CacheManager should be used for " +
101-
"annotation-driven cache management.");
102-
}
103-
this.cacheManager = cacheManagerBeans.iterator().next();
104-
// keyGenerator remains null; will fall back to default within CacheInterceptor
65+
@Autowired(required = false)
66+
void setConfigurers(Collection<C> configurers) {
67+
if (CollectionUtils.isEmpty(configurers)) {
68+
return;
10569
}
106-
if (this.cacheManager == null) {
107-
throw new IllegalStateException("No bean of type CacheManager could be found. " +
108-
"Register a CacheManager bean or remove the @EnableCaching annotation " +
109-
"from your configuration.");
70+
if (configurers.size() > 1) {
71+
throw new IllegalStateException(configurers.size() + " implementations of " +
72+
"CachingConfigurer were found when only 1 was expected. " +
73+
"Refactor the configuration such that CachingConfigurer is " +
74+
"implemented only once or not at all.");
11075
}
76+
C configurer = configurers.iterator().next();
77+
useCachingConfigurer(configurer);
11178
}
11279

11380
/**

‎spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java

+17-5
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import org.springframework.aop.framework.AopProxyUtils;
3232
import org.springframework.beans.factory.InitializingBean;
33+
import org.springframework.beans.factory.SmartInitializingSingleton;
3334
import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils;
3435
import org.springframework.cache.Cache;
3536
import org.springframework.cache.CacheManager;
@@ -72,7 +73,7 @@
7273
* @since 3.1
7374
*/
7475
public abstract class CacheAspectSupport extends AbstractCacheInvoker
75-
implements InitializingBean, ApplicationContextAware {
76+
implements InitializingBean, SmartInitializingSingleton, ApplicationContextAware {
7677

7778
protected final Log logger = LogFactory.getLog(getClass());
7879

@@ -167,15 +168,26 @@ public void setApplicationContext(ApplicationContext applicationContext) {
167168
}
168169

169170
public void afterPropertiesSet() {
170-
Assert.state(this.cacheResolver != null, "'cacheResolver' is required. Either set the cache resolver " +
171-
"to use or set the cache manager to create a default cache resolver based on it.");
172171
Assert.state(this.cacheOperationSource != null, "The 'cacheOperationSources' property is required: " +
173172
"If there are no cacheable methods, then don't use a cache aspect.");
174173
Assert.state(this.getErrorHandler() != null, "The 'errorHandler' is required.");
175-
Assert.state(this.applicationContext != null, "The application context was not injected as it should.");
176-
this.initialized = true;
177174
}
178175

176+
@Override
177+
public void afterSingletonsInstantiated() {
178+
if (getCacheResolver() == null) { // lazy initialize cache resolver
179+
CacheManager cacheManager = this.applicationContext.getBean(CacheManager.class);
180+
if (cacheManager == null) {
181+
throw new IllegalStateException("No bean of type CacheManager could be found. " +
182+
"Register a CacheManager bean or remove the @EnableCaching annotation " +
183+
"from your configuration.");
184+
}
185+
setCacheManager(cacheManager);
186+
}
187+
Assert.state(this.cacheResolver != null, "'cacheResolver' is required. Either set the cache resolver " +
188+
"to use or set the cache manager to create a default cache resolver based on it.");
189+
this.initialized = true;
190+
}
179191

180192
/**
181193
* Convenience method to return a String representation of this Method

‎spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import org.junit.Test;
2020

2121
import org.springframework.beans.factory.BeanCreationException;
22+
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
23+
import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
2224
import org.springframework.cache.CacheManager;
2325
import org.springframework.cache.CacheTestUtils;
2426
import org.springframework.cache.annotation.CachingConfigurerSupport;
@@ -73,7 +75,7 @@ public void singleCacheManagerBean() throws Throwable {
7375
ctx.refresh();
7476
}
7577

76-
@Test(expected=IllegalStateException.class)
78+
@Test(expected=NoUniqueBeanDefinitionException.class)
7779
public void multipleCacheManagerBeans() throws Throwable {
7880
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
7981
ctx.register(MultiCacheManagerConfig.class);
@@ -106,7 +108,7 @@ public void multipleCachingConfigurers() throws Throwable {
106108
}
107109
}
108110

109-
@Test(expected=IllegalStateException.class)
111+
@Test(expected=NoSuchBeanDefinitionException.class)
110112
public void noCacheManagerBeans() throws Throwable {
111113
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
112114
ctx.register(EmptyConfig.class);

0 commit comments

Comments
 (0)