From c7642422c3556e7e12279ce92bd7b8ece35cf200 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 14 Dec 2021 09:45:24 +0100 Subject: [PATCH] Stop resolving CachingConfigurer instances eagerly Closes gh-27751 --- .../AspectJEnableCachingIsolatedTests.java | 8 +- .../config/AbstractJCacheConfiguration.java | 16 ++-- .../AbstractCachingConfiguration.java | 75 ++++++++++++++----- .../cache/config/EnableCachingTests.java | 8 +- 4 files changed, 68 insertions(+), 39 deletions(-) diff --git a/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java b/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java index 7ca1037efbc4..4235f801df73 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java +++ b/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,6 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.BeanCreationException; import org.springframework.cache.CacheManager; import org.springframework.cache.annotation.CachingConfigurerSupport; import org.springframework.cache.annotation.EnableCaching; @@ -107,10 +106,7 @@ public void multipleCachingConfigurers() { try { load(MultiCacheManagerConfigurer.class, EnableCachingConfig.class); } - catch (BeanCreationException ex) { - Throwable root = ex.getRootCause(); - boolean condition = root instanceof IllegalStateException; - assertThat(condition).isTrue(); + catch (IllegalStateException ex) { assertThat(ex.getMessage().contains("implementations of CachingConfigurer")).isTrue(); } } diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/config/AbstractJCacheConfiguration.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/config/AbstractJCacheConfiguration.java index 1cd5ce612f30..4d2af4b9461b 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/config/AbstractJCacheConfiguration.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/config/AbstractJCacheConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,6 @@ import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.cache.annotation.AbstractCachingConfiguration; -import org.springframework.cache.annotation.CachingConfigurer; import org.springframework.cache.interceptor.CacheResolver; import org.springframework.cache.jcache.interceptor.DefaultJCacheOperationSource; import org.springframework.cache.jcache.interceptor.JCacheOperationSource; @@ -46,11 +45,14 @@ public abstract class AbstractJCacheConfiguration extends AbstractCachingConfigu @Override - protected void useCachingConfigurer(CachingConfigurer config) { - super.useCachingConfigurer(config); - if (config instanceof JCacheConfigurer) { - this.exceptionCacheResolver = ((JCacheConfigurer) config)::exceptionCacheResolver; - } + protected void useCachingConfigurer(CachingConfigurerSupplier cachingConfigurerSupplier) { + super.useCachingConfigurer(cachingConfigurerSupplier); + this.exceptionCacheResolver = cachingConfigurerSupplier.adapt(config -> { + if (config instanceof JCacheConfigurer) { + return ((JCacheConfigurer) config).exceptionCacheResolver(); + } + return null; + }); } @Bean(name = "jCacheOperationSource") diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/AbstractCachingConfiguration.java b/spring-context/src/main/java/org/springframework/cache/annotation/AbstractCachingConfiguration.java index c49d8d20c6e0..740ad3096b2b 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/AbstractCachingConfiguration.java +++ b/spring-context/src/main/java/org/springframework/cache/annotation/AbstractCachingConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,9 +16,12 @@ package org.springframework.cache.annotation; -import java.util.Collection; +import java.util.List; +import java.util.function.Function; import java.util.function.Supplier; +import java.util.stream.Collectors; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cache.CacheManager; import org.springframework.cache.interceptor.CacheErrorHandler; @@ -30,6 +33,7 @@ import org.springframework.core.type.AnnotationMetadata; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; +import org.springframework.util.function.SingletonSupplier; /** * Abstract base {@code @Configuration} class providing common structure @@ -70,29 +74,60 @@ public void setImportMetadata(AnnotationMetadata importMetadata) { } } - @Autowired(required = false) - void setConfigurers(Collection configurers) { - if (CollectionUtils.isEmpty(configurers)) { - return; - } - if (configurers.size() > 1) { - throw new IllegalStateException(configurers.size() + " implementations of " + - "CachingConfigurer were found when only 1 was expected. " + - "Refactor the configuration such that CachingConfigurer is " + - "implemented only once or not at all."); - } - CachingConfigurer configurer = configurers.iterator().next(); - useCachingConfigurer(configurer); + @Autowired + void setConfigurers(ObjectProvider configurers) { + Supplier cachingConfigurer = () -> { + List candidates = configurers.stream().collect(Collectors.toList()); + if (CollectionUtils.isEmpty(candidates)) { + return null; + } + if (candidates.size() > 1) { + throw new IllegalStateException(candidates.size() + " implementations of " + + "CachingConfigurer were found when only 1 was expected. " + + "Refactor the configuration such that CachingConfigurer is " + + "implemented only once or not at all."); + } + return candidates.get(0); + }; + useCachingConfigurer(new CachingConfigurerSupplier(cachingConfigurer)); } /** * Extract the configuration from the nominated {@link CachingConfigurer}. */ - protected void useCachingConfigurer(CachingConfigurer config) { - this.cacheManager = config::cacheManager; - this.cacheResolver = config::cacheResolver; - this.keyGenerator = config::keyGenerator; - this.errorHandler = config::errorHandler; + protected void useCachingConfigurer(CachingConfigurerSupplier cachingConfigurerSupplier) { + this.cacheManager = cachingConfigurerSupplier.adapt(CachingConfigurer::cacheManager); + this.cacheResolver = cachingConfigurerSupplier.adapt(CachingConfigurer::cacheResolver); + this.keyGenerator = cachingConfigurerSupplier.adapt(CachingConfigurer::keyGenerator); + this.errorHandler = cachingConfigurerSupplier.adapt(CachingConfigurer::errorHandler); + } + + + protected static class CachingConfigurerSupplier { + + private final Supplier supplier; + + public CachingConfigurerSupplier(Supplier supplier) { + this.supplier = SingletonSupplier.of(supplier); + } + + /** + * Adapt the {@link CachingConfigurer} supplier to another supplier + * provided by the specified mapping function. If the underlying + * {@link CachingConfigurer} is {@code null}, {@code null} is returned + * and the mapping function is not invoked. + * @param provider the provider to use to adapt the supplier + * @param the type of the supplier + * @return another supplier mapped by the specified function + */ + @Nullable + public Supplier adapt(Function provider) { + return () -> { + CachingConfigurer cachingConfigurer = this.supplier.get(); + return (cachingConfigurer != null) ? provider.apply(cachingConfigurer) : null; + }; + } + } } diff --git a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java index 8c41ef04570e..b7cb5f1755a8 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java +++ b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java @@ -18,7 +18,6 @@ import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.cache.CacheManager; @@ -107,11 +106,8 @@ public void multipleCachingConfigurers() { try { ctx.refresh(); } - catch (BeanCreationException ex) { - Throwable root = ex.getRootCause(); - boolean condition = root instanceof IllegalStateException; - assertThat(condition).isTrue(); - assertThat(root.getMessage().contains("implementations of CachingConfigurer")).isTrue(); + catch (IllegalStateException ex) { + assertThat(ex.getMessage().contains("implementations of CachingConfigurer")).isTrue(); } }