From 5de6ae6fca3246dd7a76f789727eb3070d2df01e Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 13 May 2020 19:25:20 +0200 Subject: [PATCH] Ignore resolved bean from non-active scope in getIfAvailable/getIfUnique Closes gh-24822 --- .../factory/support/AbstractBeanFactory.java | 8 +- .../support/DefaultListableBeanFactory.java | 106 +++++++++++++++--- .../support/ScopeNotActiveException.java | 45 ++++++++ .../context/request/RequestScopeTests.java | 45 ++++++-- .../request/RequestScopedProxyTests.java | 46 ++++++-- .../web/context/request/requestScopeTests.xml | 37 +++--- .../request/requestScopedProxyTests.xml | 4 + 7 files changed, 239 insertions(+), 52 deletions(-) create mode 100644 spring-beans/src/main/java/org/springframework/beans/factory/support/ScopeNotActiveException.java diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index 8d5d32de353..5f85a5c9d0c 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -350,6 +350,9 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp else { String scopeName = mbd.getScope(); + if (!StringUtils.hasLength(scopeName)) { + throw new IllegalStateException("No scope name defined for bean ยด" + beanName + "'"); + } final Scope scope = this.scopes.get(scopeName); if (scope == null) { throw new IllegalStateException("No Scope registered for scope name '" + scopeName + "'"); @@ -367,10 +370,7 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp bean = getObjectForBeanInstance(scopedInstance, name, beanName, mbd); } catch (IllegalStateException ex) { - throw new BeanCreationException(beanName, - "Scope '" + scopeName + "' is not active for the current thread; consider " + - "defining a scoped proxy for this bean if you intend to refer to it from a singleton", - ex); + throw new ScopeNotActiveException(beanName, scopeName, ex); } } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java index c6090bb4938..f242051cb20 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java @@ -384,12 +384,48 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @Override @Nullable public T getIfAvailable() throws BeansException { - return resolveBean(requiredType, null, false); + try { + return resolveBean(requiredType, null, false); + } + catch (ScopeNotActiveException ex) { + // Ignore resolved bean in non-active scope + return null; + } + } + @Override + public void ifAvailable(Consumer dependencyConsumer) throws BeansException { + T dependency = getIfAvailable(); + if (dependency != null) { + try { + dependencyConsumer.accept(dependency); + } + catch (ScopeNotActiveException ex) { + // Ignore resolved bean in non-active scope, even on scoped proxy invocation + } + } } @Override @Nullable public T getIfUnique() throws BeansException { - return resolveBean(requiredType, null, true); + try { + return resolveBean(requiredType, null, true); + } + catch (ScopeNotActiveException ex) { + // Ignore resolved bean in non-active scope + return null; + } + } + @Override + public void ifUnique(Consumer dependencyConsumer) throws BeansException { + T dependency = getIfUnique(); + if (dependency != null) { + try { + dependencyConsumer.accept(dependency); + } + catch (ScopeNotActiveException ex) { + // Ignore resolved bean in non-active scope, even on scoped proxy invocation + } + } } @Override public Stream stream() { @@ -1925,17 +1961,36 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @Override @Nullable public Object getIfAvailable() throws BeansException { - if (this.optional) { - return createOptionalDependency(this.descriptor, this.beanName); + try { + if (this.optional) { + return createOptionalDependency(this.descriptor, this.beanName); + } + else { + DependencyDescriptor descriptorToUse = new DependencyDescriptor(this.descriptor) { + @Override + public boolean isRequired() { + return false; + } + }; + return doResolveDependency(descriptorToUse, this.beanName, null, null); + } } - else { - DependencyDescriptor descriptorToUse = new DependencyDescriptor(this.descriptor) { - @Override - public boolean isRequired() { - return false; - } - }; - return doResolveDependency(descriptorToUse, this.beanName, null, null); + catch (ScopeNotActiveException ex) { + // Ignore resolved bean in non-active scope + return null; + } + } + + @Override + public void ifAvailable(Consumer dependencyConsumer) throws BeansException { + Object dependency = getIfAvailable(); + if (dependency != null) { + try { + dependencyConsumer.accept(dependency); + } + catch (ScopeNotActiveException ex) { + // Ignore resolved bean in non-active scope, even on scoped proxy invocation + } } } @@ -1953,11 +2008,30 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto return null; } }; - if (this.optional) { - return createOptionalDependency(descriptorToUse, this.beanName); + try { + if (this.optional) { + return createOptionalDependency(descriptorToUse, this.beanName); + } + else { + return doResolveDependency(descriptorToUse, this.beanName, null, null); + } } - else { - return doResolveDependency(descriptorToUse, this.beanName, null, null); + catch (ScopeNotActiveException ex) { + // Ignore resolved bean in non-active scope + return null; + } + } + + @Override + public void ifUnique(Consumer dependencyConsumer) throws BeansException { + Object dependency = getIfUnique(); + if (dependency != null) { + try { + dependencyConsumer.accept(dependency); + } + catch (ScopeNotActiveException ex) { + // Ignore resolved bean in non-active scope, even on scoped proxy invocation + } } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/ScopeNotActiveException.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/ScopeNotActiveException.java new file mode 100644 index 00000000000..bb7cddaf2a2 --- /dev/null +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/ScopeNotActiveException.java @@ -0,0 +1,45 @@ +/* + * Copyright 2002-2020 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.beans.factory.support; + +import org.springframework.beans.factory.BeanCreationException; + +/** + * A subclass of {@link BeanCreationException} which indicates that the target scope + * is not active, e.g. in case of request or session scope. + * + * @author Juergen Hoeller + * @since 5.3 + * @see org.springframework.beans.factory.BeanFactory#getBean + * @see org.springframework.beans.factory.config.Scope + * @see AbstractBeanDefinition#setScope + */ +@SuppressWarnings("serial") +public class ScopeNotActiveException extends BeanCreationException { + + /** + * Create a new ScopeNotActiveException. + * @param beanName the name of the bean requested + * @param scopeName the name of the target scope + * @param cause the root cause, typically from {@link org.springframework.beans.factory.config.Scope#get} + */ + public ScopeNotActiveException(String beanName, String scopeName, IllegalStateException cause) { + super(beanName, "Scope '" + scopeName + "' is not active for the current thread; consider " + + "defining a scoped proxy for this bean if you intend to refer to it from a singleton", cause); + } + +} diff --git a/spring-web/src/test/java/org/springframework/web/context/request/RequestScopeTests.java b/spring-web/src/test/java/org/springframework/web/context/request/RequestScopeTests.java index bd52c22d277..15f94f4e379 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/RequestScopeTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/RequestScopeTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -23,8 +23,11 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanCurrentlyInCreationException; import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.support.ScopeNotActiveException; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; +import org.springframework.beans.testfixture.beans.CountingTestBean; import org.springframework.beans.testfixture.beans.DerivedTestBean; import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.context.expression.StandardBeanExpressionResolver; @@ -33,6 +36,7 @@ import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.springframework.beans.factory.config.AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR; /** * @author Rob Harrop @@ -47,7 +51,7 @@ public class RequestScopeTests { @BeforeEach - public void setup() throws Exception { + public void setup() { this.beanFactory.registerScope("request", new RequestScope()); this.beanFactory.setBeanExpressionResolver(new StandardBeanExpressionResolver()); XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(this.beanFactory); @@ -62,7 +66,7 @@ public class RequestScopeTests { @Test - public void getFromScope() throws Exception { + public void getFromScope() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setContextPath("/path"); RequestAttributes requestAttributes = new ServletRequestAttributes(request); @@ -77,7 +81,7 @@ public class RequestScopeTests { } @Test - public void destructionAtRequestCompletion() throws Exception { + public void destructionAtRequestCompletion() { MockHttpServletRequest request = new MockHttpServletRequest(); ServletRequestAttributes requestAttributes = new ServletRequestAttributes(request); RequestContextHolder.setRequestAttributes(requestAttributes); @@ -93,7 +97,7 @@ public class RequestScopeTests { } @Test - public void getFromFactoryBeanInScope() throws Exception { + public void getFromFactoryBeanInScope() { MockHttpServletRequest request = new MockHttpServletRequest(); RequestAttributes requestAttributes = new ServletRequestAttributes(request); RequestContextHolder.setRequestAttributes(requestAttributes); @@ -107,7 +111,7 @@ public class RequestScopeTests { } @Test - public void circleLeadsToException() throws Exception { + public void circleLeadsToException() { MockHttpServletRequest request = new MockHttpServletRequest(); RequestAttributes requestAttributes = new ServletRequestAttributes(request); RequestContextHolder.setRequestAttributes(requestAttributes); @@ -143,7 +147,7 @@ public class RequestScopeTests { } @Test - public void requestScopedInnerBeanDestroyedWhileContainedBySingleton() throws Exception { + public void requestScopedInnerBeanDestroyedWhileContainedBySingleton() { MockHttpServletRequest request = new MockHttpServletRequest(); ServletRequestAttributes requestAttributes = new ServletRequestAttributes(request); RequestContextHolder.setRequestAttributes(requestAttributes); @@ -160,4 +164,31 @@ public class RequestScopeTests { assertThat(outer1.wasDestroyed()).isFalse(); } + @Test + public void scopeNotAvailable() { + assertThatExceptionOfType(ScopeNotActiveException.class).isThrownBy( + () -> this.beanFactory.getBean(CountingTestBean.class)); + + ObjectProvider beanProvider = this.beanFactory.getBeanProvider(CountingTestBean.class); + assertThatExceptionOfType(ScopeNotActiveException.class).isThrownBy(beanProvider::getObject); + assertThat(beanProvider.getIfAvailable()).isNull(); + assertThat(beanProvider.getIfUnique()).isNull(); + + ObjectProvider provider = + ((ProviderBean) this.beanFactory.createBean(ProviderBean.class, AUTOWIRE_CONSTRUCTOR, false)).provider; + assertThatExceptionOfType(ScopeNotActiveException.class).isThrownBy(provider::getObject); + assertThat(provider.getIfAvailable()).isNull(); + assertThat(provider.getIfUnique()).isNull(); + } + + + public static class ProviderBean { + + public ObjectProvider provider; + + public ProviderBean(ObjectProvider provider) { + this.provider = provider; + } + } + } diff --git a/spring-web/src/test/java/org/springframework/web/context/request/RequestScopedProxyTests.java b/spring-web/src/test/java/org/springframework/web/context/request/RequestScopedProxyTests.java index af46e06b94c..859808030b2 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/RequestScopedProxyTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/RequestScopedProxyTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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,10 +20,13 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.aop.support.AopUtils; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanDefinitionHolder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.support.ScopeNotActiveException; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; +import org.springframework.beans.testfixture.beans.CountingTestBean; import org.springframework.beans.testfixture.beans.DerivedTestBean; import org.springframework.beans.testfixture.beans.ITestBean; import org.springframework.beans.testfixture.beans.TestBean; @@ -32,6 +35,8 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.springframework.beans.factory.config.AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR; /** * @author Juergen Hoeller @@ -51,7 +56,7 @@ public class RequestScopedProxyTests { @Test - public void testGetFromScope() throws Exception { + public void testGetFromScope() { String name = "requestScopedObject"; TestBean bean = (TestBean) this.beanFactory.getBean(name); assertThat(AopUtils.isCglibProxy(bean)).isTrue(); @@ -76,7 +81,7 @@ public class RequestScopedProxyTests { } @Test - public void testGetFromScopeThroughDynamicProxy() throws Exception { + public void testGetFromScopeThroughDynamicProxy() { String name = "requestScopedProxy"; ITestBean bean = (ITestBean) this.beanFactory.getBean(name); // assertTrue(AopUtils.isJdkDynamicProxy(bean)); @@ -101,7 +106,7 @@ public class RequestScopedProxyTests { } @Test - public void testDestructionAtRequestCompletion() throws Exception { + public void testDestructionAtRequestCompletion() { String name = "requestScopedDisposableObject"; DerivedTestBean bean = (DerivedTestBean) this.beanFactory.getBean(name); assertThat(AopUtils.isCglibProxy(bean)).isTrue(); @@ -127,7 +132,7 @@ public class RequestScopedProxyTests { } @Test - public void testGetFromFactoryBeanInScope() throws Exception { + public void testGetFromFactoryBeanInScope() { String name = "requestScopedFactoryBean"; TestBean bean = (TestBean) this.beanFactory.getBean(name); assertThat(AopUtils.isCglibProxy(bean)).isTrue(); @@ -149,7 +154,7 @@ public class RequestScopedProxyTests { } @Test - public void testGetInnerBeanFromScope() throws Exception { + public void testGetInnerBeanFromScope() { TestBean bean = (TestBean) this.beanFactory.getBean("outerBean"); assertThat(AopUtils.isAopProxy(bean)).isFalse(); assertThat(AopUtils.isCglibProxy(bean.getSpouse())).isTrue(); @@ -173,7 +178,7 @@ public class RequestScopedProxyTests { } @Test - public void testGetAnonymousInnerBeanFromScope() throws Exception { + public void testGetAnonymousInnerBeanFromScope() { TestBean bean = (TestBean) this.beanFactory.getBean("outerBean"); assertThat(AopUtils.isAopProxy(bean)).isFalse(); assertThat(AopUtils.isCglibProxy(bean.getSpouse())).isTrue(); @@ -199,4 +204,31 @@ public class RequestScopedProxyTests { } } + @Test + public void scopeNotAvailable() { + assertThatExceptionOfType(ScopeNotActiveException.class).isThrownBy( + () -> this.beanFactory.getBean(CountingTestBean.class).absquatulate()); + + final ObjectProvider beanProvider = this.beanFactory.getBeanProvider(CountingTestBean.class); + assertThatExceptionOfType(ScopeNotActiveException.class).isThrownBy(() -> beanProvider.getObject().absquatulate()); + beanProvider.ifAvailable(TestBean::absquatulate); + beanProvider.ifUnique(TestBean::absquatulate); + + final ObjectProvider provider = + ((ProviderBean) this.beanFactory.createBean(ProviderBean.class, AUTOWIRE_CONSTRUCTOR, false)).provider; + assertThatExceptionOfType(ScopeNotActiveException.class).isThrownBy(() -> provider.getObject().absquatulate()); + provider.ifAvailable(TestBean::absquatulate); + provider.ifUnique(TestBean::absquatulate); + } + + + public static class ProviderBean { + + public ObjectProvider provider; + + public ProviderBean(ObjectProvider provider) { + this.provider = provider; + } + } + } diff --git a/spring-web/src/test/resources/org/springframework/web/context/request/requestScopeTests.xml b/spring-web/src/test/resources/org/springframework/web/context/request/requestScopeTests.xml index 826c5a87c41..9525f245047 100644 --- a/spring-web/src/test/resources/org/springframework/web/context/request/requestScopeTests.xml +++ b/spring-web/src/test/resources/org/springframework/web/context/request/requestScopeTests.xml @@ -1,8 +1,7 @@ + xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans-2.0.xsd"> @@ -12,6 +11,8 @@ + + @@ -20,22 +21,22 @@ - - - - - - - - + + + + + + + + - - - - - - - - + + + + + + + + diff --git a/spring-web/src/test/resources/org/springframework/web/context/request/requestScopedProxyTests.xml b/spring-web/src/test/resources/org/springframework/web/context/request/requestScopedProxyTests.xml index cafedde77d9..402e11c7f6b 100644 --- a/spring-web/src/test/resources/org/springframework/web/context/request/requestScopedProxyTests.xml +++ b/spring-web/src/test/resources/org/springframework/web/context/request/requestScopedProxyTests.xml @@ -24,6 +24,10 @@ + + + +