From 7ac9f92bc228f6eeea6977217a8e916f36cb1e8a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 30 Nov 2016 22:07:55 +0100 Subject: [PATCH] Various DefaultListableBeanFactory clarifications * getBeanDefinitionNames defensively returns a copy of the bean definition names array. * copyConfigurationFrom provides an independent AutowireCandidateResolver instance and copies a ConversionService and dependency comparator configuration as well. * findAutowireCandidates only considers a self reference fallback for direct dependency declarations, not as a collection element. Issue: SPR-14897 Issue: SPR-14921 Issue: SPR-14965 (cherry picked from commit ac5933a) --- .../factory/support/AbstractBeanFactory.java | 14 +++- .../support/DefaultListableBeanFactory.java | 22 +++-- .../DefaultListableBeanFactoryTests.java | 1 + ...wiredAnnotationBeanPostProcessorTests.java | 83 ++++++++++++++++--- 4 files changed, 98 insertions(+), 22 deletions(-) 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 104d91c1baa..6b034851b38 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 @@ -134,13 +134,13 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp private final Set propertyEditorRegistrars = new LinkedHashSet(4); - /** A custom TypeConverter to use, overriding the default PropertyEditor mechanism */ - private TypeConverter typeConverter; - /** Custom PropertyEditors to apply to the beans of this factory */ private final Map, Class> customEditors = new HashMap, Class>(4); + /** A custom TypeConverter to use, overriding the default PropertyEditor mechanism */ + private TypeConverter typeConverter; + /** String resolvers to apply e.g. to annotation attribute values */ private final List embeddedValueResolvers = new LinkedList(); @@ -921,10 +921,12 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp setBeanClassLoader(otherFactory.getBeanClassLoader()); setCacheBeanMetadata(otherFactory.isCacheBeanMetadata()); setBeanExpressionResolver(otherFactory.getBeanExpressionResolver()); + setConversionService(otherFactory.getConversionService()); if (otherFactory instanceof AbstractBeanFactory) { AbstractBeanFactory otherAbstractFactory = (AbstractBeanFactory) otherFactory; - this.customEditors.putAll(otherAbstractFactory.customEditors); this.propertyEditorRegistrars.addAll(otherAbstractFactory.propertyEditorRegistrars); + this.customEditors.putAll(otherAbstractFactory.customEditors); + this.typeConverter = otherAbstractFactory.typeConverter; this.beanPostProcessors.addAll(otherAbstractFactory.beanPostProcessors); this.hasInstantiationAwareBeanPostProcessors = this.hasInstantiationAwareBeanPostProcessors || otherAbstractFactory.hasInstantiationAwareBeanPostProcessors; @@ -935,6 +937,10 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp } else { setTypeConverter(otherFactory.getTypeConverter()); + String[] otherScopeNames = otherFactory.getRegisteredScopeNames(); + for (String scopeName : otherScopeNames) { + this.scopes.put(scopeName, otherFactory.getRegisteredScope(scopeName)); + } } } 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 c85ef3ca0cf..28ce98f01af 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 @@ -43,6 +43,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import javax.inject.Provider; +import org.springframework.beans.BeanUtils; import org.springframework.beans.BeansException; import org.springframework.beans.TypeConverter; import org.springframework.beans.factory.BeanCreationException; @@ -266,6 +267,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto /** * Set a {@link java.util.Comparator} for dependency Lists and arrays. + * @since 4.0 * @see org.springframework.core.OrderComparator * @see org.springframework.core.annotation.AnnotationAwareOrderComparator */ @@ -275,6 +277,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto /** * Return the dependency comparator for this BeanFactory (may be {@code null}. + * @since 4.0 */ public Comparator getDependencyComparator() { return this.dependencyComparator; @@ -289,11 +292,10 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto Assert.notNull(autowireCandidateResolver, "AutowireCandidateResolver must not be null"); if (autowireCandidateResolver instanceof BeanFactoryAware) { if (System.getSecurityManager() != null) { - final BeanFactory target = this; AccessController.doPrivileged(new PrivilegedAction() { @Override public Object run() { - ((BeanFactoryAware) autowireCandidateResolver).setBeanFactory(target); + ((BeanFactoryAware) autowireCandidateResolver).setBeanFactory(DefaultListableBeanFactory.this); return null; } }, getAccessControlContext()); @@ -320,7 +322,10 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto DefaultListableBeanFactory otherListableFactory = (DefaultListableBeanFactory) otherFactory; this.allowBeanDefinitionOverriding = otherListableFactory.allowBeanDefinitionOverriding; this.allowEagerClassLoading = otherListableFactory.allowEagerClassLoading; - this.autowireCandidateResolver = otherListableFactory.autowireCandidateResolver; + this.dependencyComparator = otherListableFactory.dependencyComparator; + // A clone of the AutowireCandidateResolver since it is potentially BeanFactoryAware... + setAutowireCandidateResolver(BeanUtils.instantiateClass(getAutowireCandidateResolver().getClass())); + // Make resolvable dependencies (e.g. ResourceLoader) available here as well... this.resolvableDependencies.putAll(otherListableFactory.resolvableDependencies); } } @@ -367,7 +372,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @Override public String[] getBeanDefinitionNames() { if (this.frozenBeanDefinitionNames != null) { - return this.frozenBeanDefinitionNames; + return this.frozenBeanDefinitionNames.clone(); } else { return StringUtils.toStringArray(this.beanDefinitionNames); @@ -1266,8 +1271,9 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto addCandidateEntry(result, candidateName, descriptor, requiredType); } } - if (result.isEmpty()) { - // Consider self references before as a final pass + if (result.isEmpty() && !(descriptor instanceof MultiElementDependencyDescriptor)) { + // Consider self references as a final pass... + // but not as collection elements, just for direct dependency declarations. for (String candidateName : candidateNames) { if (isSelfReference(beanName, candidateName) && isAutowireCandidate(candidateName, fallbackDescriptor)) { addCandidateEntry(result, candidateName, descriptor, requiredType); @@ -1476,10 +1482,10 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto Class targetType = mbd.getTargetType(); if (targetType != null && type.isAssignableFrom(targetType) && isAutowireCandidate(beanName, mbd, descriptor, getAutowireCandidateResolver())) { - // Probably a poxy interfering with target type match -> throw meaningful exception. + // Probably a proxy interfering with target type match -> throw meaningful exception. Object beanInstance = getSingleton(beanName, false); Class beanType = (beanInstance != null ? beanInstance.getClass() : predictBeanType(beanName, mbd)); - if (type != beanType) { + if (!type.isAssignableFrom((beanType))) { throw new BeanNotOfRequiredTypeException(beanName, type, beanType); } } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index 17f35614c1b..fb0c1b408b0 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java @@ -776,6 +776,7 @@ public class DefaultListableBeanFactoryTests { private void testSingleTestBean(ListableBeanFactory lbf) { assertTrue("1 beans defined", lbf.getBeanDefinitionCount() == 1); String[] names = lbf.getBeanDefinitionNames(); + assertTrue(names != lbf.getBeanDefinitionNames()); assertTrue("Array length == 1", names.length == 1); assertTrue("0th element == test", names[0].equals("test")); TestBean tb = (TestBean) lbf.getBean("test"); diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java index 4e33a0bbb6e..ec83ce05899 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java @@ -29,6 +29,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Properties; @@ -920,7 +921,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { RootBeanDefinition tbm = new RootBeanDefinition(CollectionFactoryMethods.class); tbm.setUniqueFactoryMethodName("testBeanMap"); bf.registerBeanDefinition("myTestBeanMap", tbm); - bf.registerSingleton("otherMap", new HashMap()); + bf.registerSingleton("otherMap", new HashMap<>()); MapConstructorInjectionBean bean = (MapConstructorInjectionBean) bf.getBean("annotatedBean"); assertSame(bf.getBean("myTestBeanMap"), bean.getTestBeanMap()); @@ -942,7 +943,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { tbs.add(new TestBean("tb1")); tbs.add(new TestBean("tb2")); bf.registerSingleton("testBeans", tbs); - bf.registerSingleton("otherSet", new HashSet()); + bf.registerSingleton("otherSet", new HashSet<>()); SetConstructorInjectionBean bean = (SetConstructorInjectionBean) bf.getBean("annotatedBean"); assertSame(tbs, bean.getTestBeanSet()); @@ -963,7 +964,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { RootBeanDefinition tbs = new RootBeanDefinition(CollectionFactoryMethods.class); tbs.setUniqueFactoryMethodName("testBeanSet"); bf.registerBeanDefinition("myTestBeanSet", tbs); - bf.registerSingleton("otherSet", new HashSet()); + bf.registerSingleton("otherSet", new HashSet<>()); SetConstructorInjectionBean bean = (SetConstructorInjectionBean) bf.getBean("annotatedBean"); assertSame(bf.getBean("myTestBeanSet"), bean.getTestBeanSet()); @@ -978,11 +979,59 @@ public class AutowiredAnnotationBeanPostProcessorTests { AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); bpp.setBeanFactory(bf); bf.addBeanPostProcessor(bpp); - RootBeanDefinition bd = new RootBeanDefinition(SelfInjectionBean.class); - bf.registerBeanDefinition("annotatedBean", bd); + bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(SelfInjectionBean.class)); + + SelfInjectionBean bean = (SelfInjectionBean) bf.getBean("annotatedBean"); + assertSame(bean, bean.reference); + assertNull(bean.referenceCollection); + } + + @Test + public void testSelfReferenceWithOther() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); + AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); + bpp.setBeanFactory(bf); + bf.addBeanPostProcessor(bpp); + bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(SelfInjectionBean.class)); + bf.registerBeanDefinition("annotatedBean2", new RootBeanDefinition(SelfInjectionBean.class)); SelfInjectionBean bean = (SelfInjectionBean) bf.getBean("annotatedBean"); - assertSame(bean, bean.selfReference); + SelfInjectionBean bean2 = (SelfInjectionBean) bf.getBean("annotatedBean2"); + assertSame(bean2, bean.reference); + assertEquals(1, bean.referenceCollection.size()); + assertSame(bean2, bean.referenceCollection.get(0)); + } + + @Test + public void testSelfReferenceCollection() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); + AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); + bpp.setBeanFactory(bf); + bf.addBeanPostProcessor(bpp); + bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(SelfInjectionCollectionBean.class)); + + SelfInjectionCollectionBean bean = (SelfInjectionCollectionBean) bf.getBean("annotatedBean"); + assertSame(bean, bean.reference); + assertNull(bean.referenceCollection); + } + + @Test + public void testSelfReferenceCollectionWithOther() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); + AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); + bpp.setBeanFactory(bf); + bf.addBeanPostProcessor(bpp); + bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(SelfInjectionCollectionBean.class)); + bf.registerBeanDefinition("annotatedBean2", new RootBeanDefinition(SelfInjectionCollectionBean.class)); + + SelfInjectionCollectionBean bean = (SelfInjectionCollectionBean) bf.getBean("annotatedBean"); + SelfInjectionCollectionBean bean2 = (SelfInjectionCollectionBean) bf.getBean("annotatedBean2"); + assertSame(bean2, bean.reference); + assertSame(1, bean2.referenceCollection.size()); + assertSame(bean2, bean.referenceCollection.get(0)); } @Test @@ -2582,7 +2631,21 @@ public class AutowiredAnnotationBeanPostProcessorTests { public static class SelfInjectionBean { @Autowired - public SelfInjectionBean selfReference; + public SelfInjectionBean reference; + + @Autowired(required = false) + public List referenceCollection; + } + + + @SuppressWarnings("serial") + public static class SelfInjectionCollectionBean extends LinkedList { + + @Autowired + public SelfInjectionCollectionBean reference; + + @Autowired(required = false) + public List referenceCollection; } @@ -3149,7 +3212,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { } public static GenericInterface1 createErased() { - return new GenericInterface1Impl(); + return new GenericInterface1Impl<>(); } @SuppressWarnings("rawtypes") @@ -3336,14 +3399,14 @@ public class AutowiredAnnotationBeanPostProcessorTests { public static class CollectionFactoryMethods { public static Map testBeanMap() { - Map tbm = new LinkedHashMap(); + Map tbm = new LinkedHashMap<>(); tbm.put("testBean1", new TestBean("tb1")); tbm.put("testBean2", new TestBean("tb2")); return tbm; } public static Set testBeanSet() { - Set tbs = new LinkedHashSet(); + Set tbs = new LinkedHashSet<>(); tbs.add(new TestBean("tb1")); tbs.add(new TestBean("tb2")); return tbs;