From 2f3571f173079a490d447c3fe87bd98045d37a76 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 30 Dec 2016 14:19:03 +0100 Subject: [PATCH 1/2] Defend against lambda transaction customizers Update `CacheManagerCustomizers` to deal directly with `ClassCastException` assuming that they are because a customizer is implemented using a lambda. Closes gh-7788 --- .../cache/CacheManagerCustomizers.java | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizers.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizers.java index a821e762764..fb76047a0d8 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizers.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizers.java @@ -21,6 +21,9 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.cache.CacheManager; @@ -38,6 +41,8 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator; */ class CacheManagerCustomizers implements ApplicationContextAware { + private static final Log logger = LogFactory.getLog(CacheManagerCustomizers.class); + private ConfigurableApplicationContext applicationContext; /** @@ -53,11 +58,27 @@ class CacheManagerCustomizers implements ApplicationContextAware { cacheManager); AnnotationAwareOrderComparator.sort(customizers); for (CacheManagerCustomizer customizer : customizers) { - customizer.customize(cacheManager); + customize(cacheManager, customizer); } return cacheManager; } + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void customize(CacheManager cacheManager, + CacheManagerCustomizer customizer) { + try { + customizer.customize(cacheManager); + } + catch (ClassCastException ex) { + // Possibly a lambda-defined listener which we could not resolve the generic + // event type for + if (logger.isDebugEnabled()) { + logger.debug("Non-matching transaction manager type for customizer: " + + customizer, ex); + } + } + } + @SuppressWarnings({ "unchecked", "rawtypes" }) private List> findCustomizers( CacheManager cacheManager) { From c230a21a8c5616e215bf6195f7b47a6a0dc8e5f2 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 30 Dec 2016 15:15:31 +0100 Subject: [PATCH 2/2] Polish CacheManagerCustomizers Closes gh-7792 --- .../cache/CacheAutoConfiguration.java | 12 ++- .../cache/CacheManagerCustomizers.java | 73 +++++---------- .../cache/CacheManagerCustomizersTests.java | 90 ++++++++++--------- 3 files changed, 79 insertions(+), 96 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheAutoConfiguration.java index f30dc360674..71900d6fb99 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheAutoConfiguration.java @@ -16,9 +16,12 @@ package org.springframework.boot.autoconfigure.cache; +import java.util.List; + import javax.annotation.PostConstruct; import org.springframework.beans.BeansException; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; @@ -67,11 +70,18 @@ import org.springframework.util.Assert; @AutoConfigureBefore(HibernateJpaAutoConfiguration.class) @AutoConfigureAfter({ CouchbaseAutoConfiguration.class, HazelcastAutoConfiguration.class, RedisAutoConfiguration.class }) -@Import({ CacheManagerCustomizers.class, CacheConfigurationImportSelector.class }) +@Import(CacheConfigurationImportSelector.class) public class CacheAutoConfiguration { static final String VALIDATOR_BEAN_NAME = "cacheAutoConfigurationValidator"; + @Bean + @ConditionalOnMissingBean + public CacheManagerCustomizers cacheManagerCustomizers( + ObjectProvider>> customizers) { + return new CacheManagerCustomizers(customizers.getIfAvailable()); + } + @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) public static CacheManagerValidatorPostProcessor cacheAutoConfigurationValidatorPostProcessor() { diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizers.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizers.java index fb76047a0d8..c9a911af35b 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizers.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizers.java @@ -17,33 +17,34 @@ package org.springframework.boot.autoconfigure.cache; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.beans.BeansException; -import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.cache.CacheManager; -import org.springframework.context.ApplicationContext; -import org.springframework.context.ApplicationContextAware; -import org.springframework.context.ConfigurableApplicationContext; -import org.springframework.core.GenericTypeResolver; -import org.springframework.core.annotation.AnnotationAwareOrderComparator; +import org.springframework.core.ResolvableType; /** * Invokes the available {@link CacheManagerCustomizer} instances in the context for a * given {@link CacheManager}. * * @author Stephane Nicoll + * @since 1.5.0 */ -class CacheManagerCustomizers implements ApplicationContextAware { +public class CacheManagerCustomizers { private static final Log logger = LogFactory.getLog(CacheManagerCustomizers.class); - private ConfigurableApplicationContext applicationContext; + private final List> customizers; + + public CacheManagerCustomizers( + List> customizers) { + this.customizers = (customizers != null ? + new ArrayList>(customizers) : + Collections.>emptyList()); + } /** * Customize the specified {@link CacheManager}. Locates all @@ -54,18 +55,20 @@ class CacheManagerCustomizers implements ApplicationContextAware { * @return the cache manager */ public T customize(T cacheManager) { - List> customizers = findCustomizers( - cacheManager); - AnnotationAwareOrderComparator.sort(customizers); - for (CacheManagerCustomizer customizer : customizers) { - customize(cacheManager, customizer); + for (CacheManagerCustomizer customizer : this.customizers) { + Class generic = ResolvableType + .forClass(CacheManagerCustomizer.class, + customizer.getClass()) + .resolveGeneric(); + if (generic.isInstance(cacheManager)) { + customize(cacheManager, customizer); + } } return cacheManager; } @SuppressWarnings({ "unchecked", "rawtypes" }) - private void customize(CacheManager cacheManager, - CacheManagerCustomizer customizer) { + private void customize(CacheManager cacheManager, CacheManagerCustomizer customizer) { try { customizer.customize(cacheManager); } @@ -79,40 +82,4 @@ class CacheManagerCustomizers implements ApplicationContextAware { } } - @SuppressWarnings({ "unchecked", "rawtypes" }) - private List> findCustomizers( - CacheManager cacheManager) { - if (this.applicationContext == null) { - return Collections.emptyList(); - } - Class cacheManagerClass = cacheManager.getClass(); - List> customizers = new ArrayList>(); - for (CacheManagerCustomizer customizer : getBeans(CacheManagerCustomizer.class)) { - if (canCustomize(customizer, cacheManagerClass)) { - customizers.add(customizer); - } - } - return customizers; - } - - private Collection getBeans(Class type) { - return BeanFactoryUtils.beansOfTypeIncludingAncestors( - this.applicationContext.getBeanFactory(), type).values(); - } - - private boolean canCustomize(CacheManagerCustomizer customizer, - Class cacheManagerClass) { - Class target = GenericTypeResolver.resolveTypeArgument(customizer.getClass(), - CacheManagerCustomizer.class); - return (target == null || target.isAssignableFrom(cacheManagerClass)); - } - - @Override - public void setApplicationContext(ApplicationContext applicationContext) - throws BeansException { - if (applicationContext instanceof ConfigurableApplicationContext) { - this.applicationContext = (ConfigurableApplicationContext) applicationContext; - } - } - } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizersTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizersTests.java index eef34e9a64e..2a1d6476454 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizersTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cache/CacheManagerCustomizersTests.java @@ -16,80 +16,86 @@ package org.springframework.boot.autoconfigure.cache; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.List; -import org.junit.After; import org.junit.Test; -import org.springframework.boot.test.util.EnvironmentTestUtils; import org.springframework.cache.CacheManager; -import org.springframework.cache.annotation.EnableCaching; +import org.springframework.cache.caffeine.CaffeineCacheManager; import org.springframework.cache.concurrent.ConcurrentMapCacheManager; -import org.springframework.context.ApplicationContext; -import org.springframework.context.annotation.AnnotationConfigApplicationContext; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verifyZeroInteractions; /** * @author Stephane Nicoll */ public class CacheManagerCustomizersTests { - private AnnotationConfigApplicationContext context; - - @After - public void tearDown() { - if (this.context != null) { - this.context.close(); - } + @Test + public void customizeWithNullCustomizersShouldDoNothing() { + new CacheManagerCustomizers(null) + .customize(mock(CacheManager.class)); } @Test public void customizeSimpleCacheManager() { - load(SimpleConfiguration.class, "spring.cache.type=simple"); - ConcurrentMapCacheManager cacheManager = this.context - .getBean(ConcurrentMapCacheManager.class); + CacheManagerCustomizers customizers = new CacheManagerCustomizers( + Collections.singletonList(new CacheNamesCacheManagerCustomizer())); + ConcurrentMapCacheManager cacheManager = new ConcurrentMapCacheManager(); + customizers.customize(cacheManager); assertThat(cacheManager.getCacheNames()).containsOnly("one", "two"); } @Test - public void customizeNoConfigurableApplicationContext() { - CacheManagerCustomizers invoker = new CacheManagerCustomizers(); - ApplicationContext context = mock(ApplicationContext.class); - invoker.setApplicationContext(context); - invoker.customize(mock(CacheManager.class)); - verifyZeroInteractions(context); + public void customizeShouldCheckGeneric() throws Exception { + List> list = new ArrayList>(); + list.add(new TestCustomizer()); + list.add(new TestConcurrentMapCacheManagerCustomizer()); + CacheManagerCustomizers customizers = new CacheManagerCustomizers(list); + customizers.customize(mock(CacheManager.class)); + assertThat(list.get(0).getCount()).isEqualTo(1); + assertThat(list.get(1).getCount()).isEqualTo(0); + customizers.customize(mock(ConcurrentMapCacheManager.class)); + assertThat(list.get(0).getCount()).isEqualTo(2); + assertThat(list.get(1).getCount()).isEqualTo(1); + customizers.customize(mock(CaffeineCacheManager.class)); + assertThat(list.get(0).getCount()).isEqualTo(3); + assertThat(list.get(1).getCount()).isEqualTo(1); } - private void load(Class config, String... environment) { - AnnotationConfigApplicationContext applicationContext = new AnnotationConfigApplicationContext(); - EnvironmentTestUtils.addEnvironment(applicationContext, environment); - applicationContext.register(config); - applicationContext.register(CacheAutoConfiguration.class); - applicationContext.refresh(); - this.context = applicationContext; + static class CacheNamesCacheManagerCustomizer + implements CacheManagerCustomizer { + + @Override + public void customize(ConcurrentMapCacheManager cacheManager) { + cacheManager.setCacheNames(Arrays.asList("one", "two")); + } + } - @Configuration - @EnableCaching - static class SimpleConfiguration { + private static class TestCustomizer + implements CacheManagerCustomizer { - @Bean - public CacheManagerCustomizer cacheManagerCustomizer() { - return new CacheManagerCustomizer() { + private int count; - @Override - public void customize(ConcurrentMapCacheManager cacheManager) { - cacheManager.setCacheNames(Arrays.asList("one", "two")); - } + @Override + public void customize(T cacheManager) { + this.count++; + } - }; + public int getCount() { + return this.count; } } + private static class TestConcurrentMapCacheManagerCustomizer + extends TestCustomizer { + + } + }