From d8d8f9cf0b1eafe228195d334c218fe9488f47ba Mon Sep 17 00:00:00 2001 From: Vlad Kisel Date: Mon, 13 Apr 2020 12:04:17 +0200 Subject: [PATCH 1/2] Allow beans without public constructors to load Allow `BeanDefinitionLoader` to load classes that don't have public constructors. The constraint was first introduced in d82c50804f04 to solve an issue with anonymous Groovy classes but causes particular problems with `@SpringBootTest`. See gh-20929 --- .../boot/BeanDefinitionLoader.java | 22 ++++++--------- .../boot/BeanDefinitionLoaderTests.java | 26 +++++++++++++++-- .../boot/sampleconfig/MyNamedComponent.java | 28 +++++++++++++++++++ 3 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sampleconfig/MyNamedComponent.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/BeanDefinitionLoader.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/BeanDefinitionLoader.java index 760a436ac7..97a6752dcd 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/BeanDefinitionLoader.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/BeanDefinitionLoader.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-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. @@ -31,8 +31,6 @@ import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; import org.springframework.context.annotation.AnnotatedBeanDefinitionReader; import org.springframework.context.annotation.ClassPathBeanDefinitionScanner; -import org.springframework.core.annotation.MergedAnnotations; -import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; @@ -41,7 +39,6 @@ import org.springframework.core.io.support.PathMatchingResourcePatternResolver; import org.springframework.core.io.support.ResourcePatternResolver; import org.springframework.core.type.filter.AbstractTypeHierarchyTraversingFilter; import org.springframework.core.type.filter.TypeFilter; -import org.springframework.stereotype.Component; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; @@ -53,6 +50,7 @@ import org.springframework.util.StringUtils; * {@link SpringApplication} for the types of sources that are supported. * * @author Phillip Webb + * @author Vladislav Kisel * @see #setBeanNameGenerator(BeanNameGenerator) */ class BeanDefinitionLoader { @@ -273,16 +271,14 @@ class BeanDefinitionLoader { return Package.getPackage(source.toString()); } + /** + * Check whether the bean is eligible for registration. + * @param type candidate bean type + * @return true if the given bean type is eligible for registration, i.e. not a groovy + * closure nor an anonymous class + */ private boolean isComponent(Class type) { - // This has to be a bit of a guess. The only way to be sure that this type is - // eligible is to make a bean definition out of it and try to instantiate it. - if (MergedAnnotations.from(type, SearchStrategy.TYPE_HIERARCHY).isPresent(Component.class)) { - return true; - } - // Nested anonymous classes are not eligible for registration, nor are groovy - // closures - return !type.getName().matches(".*\\$_.*closure.*") && !type.isAnonymousClass() - && type.getConstructors() != null && type.getConstructors().length != 0; + return !type.getName().matches(".*\\$_.*closure.*") && !type.isAnonymousClass(); } /** diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BeanDefinitionLoaderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BeanDefinitionLoaderTests.java index d194df1fbb..6d1f0f2814 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BeanDefinitionLoaderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BeanDefinitionLoaderTests.java @@ -22,6 +22,7 @@ import org.junit.jupiter.api.Test; import sampleconfig.MyComponentInPackageWithoutDot; import org.springframework.boot.sampleconfig.MyComponent; +import org.springframework.boot.sampleconfig.MyNamedComponent; import org.springframework.context.support.StaticApplicationContext; import org.springframework.core.io.ClassPathResource; @@ -31,6 +32,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link BeanDefinitionLoader}. * * @author Phillip Webb + * @author Vladislav Kisel */ class BeanDefinitionLoaderTests { @@ -53,6 +55,21 @@ class BeanDefinitionLoaderTests { assertThat(this.registry.containsBean("myComponent")).isTrue(); } + @Test + void anonymousClassNotLoaded() { + MyComponent myComponent = new MyComponent() { + }; + BeanDefinitionLoader loader = new BeanDefinitionLoader(this.registry, myComponent.getClass()); + assertThat(loader.load()).isEqualTo(0); + } + + @Test + void loadJsr330Class() { + BeanDefinitionLoader loader = new BeanDefinitionLoader(this.registry, MyNamedComponent.class); + assertThat(loader.load()).isEqualTo(1); + assertThat(this.registry.containsBean("myNamedComponent")).isTrue(); + } + @Test void loadXmlResource() { ClassPathResource resource = new ClassPathResource("sample-beans.xml", getClass()); @@ -83,8 +100,9 @@ class BeanDefinitionLoaderTests { @Test void loadPackage() { BeanDefinitionLoader loader = new BeanDefinitionLoader(this.registry, MyComponent.class.getPackage()); - assertThat(loader.load()).isEqualTo(1); + assertThat(loader.load()).isEqualTo(2); assertThat(this.registry.containsBean("myComponent")).isTrue(); + assertThat(this.registry.containsBean("myNamedComponent")).isTrue(); } @Test @@ -113,8 +131,9 @@ class BeanDefinitionLoaderTests { @Test void loadPackageName() { BeanDefinitionLoader loader = new BeanDefinitionLoader(this.registry, MyComponent.class.getPackage().getName()); - assertThat(loader.load()).isEqualTo(1); + assertThat(loader.load()).isEqualTo(2); assertThat(this.registry.containsBean("myComponent")).isTrue(); + assertThat(this.registry.containsBean("myNamedComponent")).isTrue(); } @Test @@ -131,8 +150,9 @@ class BeanDefinitionLoaderTests { void loadPackageAndClassDoesNotDoubleAdd() { BeanDefinitionLoader loader = new BeanDefinitionLoader(this.registry, MyComponent.class.getPackage(), MyComponent.class); - assertThat(loader.load()).isEqualTo(1); + assertThat(loader.load()).isEqualTo(2); assertThat(this.registry.containsBean("myComponent")).isTrue(); + assertThat(this.registry.containsBean("myNamedComponent")).isTrue(); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sampleconfig/MyNamedComponent.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sampleconfig/MyNamedComponent.java new file mode 100644 index 0000000000..f04a455db7 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sampleconfig/MyNamedComponent.java @@ -0,0 +1,28 @@ +/* + * Copyright 2012-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.boot.sampleconfig; + +import javax.inject.Named; + +@Named +public class MyNamedComponent { + + MyNamedComponent() { + + } + +} From c11abf48d9744e9af48417fe41a5459af955c3fd Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sat, 6 Jun 2020 11:59:36 -0700 Subject: [PATCH 2/2] Polish 'Allow beans without public constructors to load' See gh-20929 --- .../boot/BeanDefinitionLoader.java | 17 ++++++++++++++--- .../boot/BeanDefinitionLoaderTests.java | 1 + .../boot/sampleconfig/MyNamedComponent.java | 1 - 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/BeanDefinitionLoader.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/BeanDefinitionLoader.java index 97a6752dcd..4627965ee6 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/BeanDefinitionLoader.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/BeanDefinitionLoader.java @@ -17,6 +17,7 @@ package org.springframework.boot; import java.io.IOException; +import java.lang.reflect.Constructor; import java.util.HashSet; import java.util.Set; @@ -41,6 +42,7 @@ import org.springframework.core.type.filter.AbstractTypeHierarchyTraversingFilte import org.springframework.core.type.filter.TypeFilter; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -151,7 +153,7 @@ class BeanDefinitionLoader { GroovyBeanDefinitionSource loader = BeanUtils.instantiateClass(source, GroovyBeanDefinitionSource.class); load(loader); } - if (isComponent(source)) { + if (isEligible(source)) { this.annotatedReader.register(source); return 1; } @@ -277,8 +279,17 @@ class BeanDefinitionLoader { * @return true if the given bean type is eligible for registration, i.e. not a groovy * closure nor an anonymous class */ - private boolean isComponent(Class type) { - return !type.getName().matches(".*\\$_.*closure.*") && !type.isAnonymousClass(); + private boolean isEligible(Class type) { + return !(type.isAnonymousClass() || isGroovyClosure(type) || hasNoConstructors(type)); + } + + private boolean isGroovyClosure(Class type) { + return type.getName().matches(".*\\$_.*closure.*"); + } + + private boolean hasNoConstructors(Class type) { + Constructor[] constructors = type.getDeclaredConstructors(); + return ObjectUtils.isEmpty(constructors); } /** diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BeanDefinitionLoaderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BeanDefinitionLoaderTests.java index 6d1f0f2814..7ddf590749 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BeanDefinitionLoaderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BeanDefinitionLoaderTests.java @@ -58,6 +58,7 @@ class BeanDefinitionLoaderTests { @Test void anonymousClassNotLoaded() { MyComponent myComponent = new MyComponent() { + }; BeanDefinitionLoader loader = new BeanDefinitionLoader(this.registry, myComponent.getClass()); assertThat(loader.load()).isEqualTo(0); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sampleconfig/MyNamedComponent.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sampleconfig/MyNamedComponent.java index f04a455db7..662c3869ec 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sampleconfig/MyNamedComponent.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sampleconfig/MyNamedComponent.java @@ -22,7 +22,6 @@ import javax.inject.Named; public class MyNamedComponent { MyNamedComponent() { - } }