Skip to content

Commit 7ac9f92

Browse files
committed
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)
1 parent a7ba63d commit 7ac9f92

File tree

4 files changed

+98
-22
lines changed

4 files changed

+98
-22
lines changed

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java

+10-4
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,13 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp
134134
private final Set<PropertyEditorRegistrar> propertyEditorRegistrars =
135135
new LinkedHashSet<PropertyEditorRegistrar>(4);
136136

137-
/** A custom TypeConverter to use, overriding the default PropertyEditor mechanism */
138-
private TypeConverter typeConverter;
139-
140137
/** Custom PropertyEditors to apply to the beans of this factory */
141138
private final Map<Class<?>, Class<? extends PropertyEditor>> customEditors =
142139
new HashMap<Class<?>, Class<? extends PropertyEditor>>(4);
143140

141+
/** A custom TypeConverter to use, overriding the default PropertyEditor mechanism */
142+
private TypeConverter typeConverter;
143+
144144
/** String resolvers to apply e.g. to annotation attribute values */
145145
private final List<StringValueResolver> embeddedValueResolvers = new LinkedList<StringValueResolver>();
146146

@@ -921,10 +921,12 @@ public void copyConfigurationFrom(ConfigurableBeanFactory otherFactory) {
921921
setBeanClassLoader(otherFactory.getBeanClassLoader());
922922
setCacheBeanMetadata(otherFactory.isCacheBeanMetadata());
923923
setBeanExpressionResolver(otherFactory.getBeanExpressionResolver());
924+
setConversionService(otherFactory.getConversionService());
924925
if (otherFactory instanceof AbstractBeanFactory) {
925926
AbstractBeanFactory otherAbstractFactory = (AbstractBeanFactory) otherFactory;
926-
this.customEditors.putAll(otherAbstractFactory.customEditors);
927927
this.propertyEditorRegistrars.addAll(otherAbstractFactory.propertyEditorRegistrars);
928+
this.customEditors.putAll(otherAbstractFactory.customEditors);
929+
this.typeConverter = otherAbstractFactory.typeConverter;
928930
this.beanPostProcessors.addAll(otherAbstractFactory.beanPostProcessors);
929931
this.hasInstantiationAwareBeanPostProcessors = this.hasInstantiationAwareBeanPostProcessors ||
930932
otherAbstractFactory.hasInstantiationAwareBeanPostProcessors;
@@ -935,6 +937,10 @@ public void copyConfigurationFrom(ConfigurableBeanFactory otherFactory) {
935937
}
936938
else {
937939
setTypeConverter(otherFactory.getTypeConverter());
940+
String[] otherScopeNames = otherFactory.getRegisteredScopeNames();
941+
for (String scopeName : otherScopeNames) {
942+
this.scopes.put(scopeName, otherFactory.getRegisteredScope(scopeName));
943+
}
938944
}
939945
}
940946

spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java

+14-8
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import java.util.concurrent.ConcurrentHashMap;
4444
import javax.inject.Provider;
4545

46+
import org.springframework.beans.BeanUtils;
4647
import org.springframework.beans.BeansException;
4748
import org.springframework.beans.TypeConverter;
4849
import org.springframework.beans.factory.BeanCreationException;
@@ -266,6 +267,7 @@ public boolean isAllowEagerClassLoading() {
266267

267268
/**
268269
* Set a {@link java.util.Comparator} for dependency Lists and arrays.
270+
* @since 4.0
269271
* @see org.springframework.core.OrderComparator
270272
* @see org.springframework.core.annotation.AnnotationAwareOrderComparator
271273
*/
@@ -275,6 +277,7 @@ public void setDependencyComparator(Comparator<Object> dependencyComparator) {
275277

276278
/**
277279
* Return the dependency comparator for this BeanFactory (may be {@code null}.
280+
* @since 4.0
278281
*/
279282
public Comparator<Object> getDependencyComparator() {
280283
return this.dependencyComparator;
@@ -289,11 +292,10 @@ public void setAutowireCandidateResolver(final AutowireCandidateResolver autowir
289292
Assert.notNull(autowireCandidateResolver, "AutowireCandidateResolver must not be null");
290293
if (autowireCandidateResolver instanceof BeanFactoryAware) {
291294
if (System.getSecurityManager() != null) {
292-
final BeanFactory target = this;
293295
AccessController.doPrivileged(new PrivilegedAction<Object>() {
294296
@Override
295297
public Object run() {
296-
((BeanFactoryAware) autowireCandidateResolver).setBeanFactory(target);
298+
((BeanFactoryAware) autowireCandidateResolver).setBeanFactory(DefaultListableBeanFactory.this);
297299
return null;
298300
}
299301
}, getAccessControlContext());
@@ -320,7 +322,10 @@ public void copyConfigurationFrom(ConfigurableBeanFactory otherFactory) {
320322
DefaultListableBeanFactory otherListableFactory = (DefaultListableBeanFactory) otherFactory;
321323
this.allowBeanDefinitionOverriding = otherListableFactory.allowBeanDefinitionOverriding;
322324
this.allowEagerClassLoading = otherListableFactory.allowEagerClassLoading;
323-
this.autowireCandidateResolver = otherListableFactory.autowireCandidateResolver;
325+
this.dependencyComparator = otherListableFactory.dependencyComparator;
326+
// A clone of the AutowireCandidateResolver since it is potentially BeanFactoryAware...
327+
setAutowireCandidateResolver(BeanUtils.instantiateClass(getAutowireCandidateResolver().getClass()));
328+
// Make resolvable dependencies (e.g. ResourceLoader) available here as well...
324329
this.resolvableDependencies.putAll(otherListableFactory.resolvableDependencies);
325330
}
326331
}
@@ -367,7 +372,7 @@ public int getBeanDefinitionCount() {
367372
@Override
368373
public String[] getBeanDefinitionNames() {
369374
if (this.frozenBeanDefinitionNames != null) {
370-
return this.frozenBeanDefinitionNames;
375+
return this.frozenBeanDefinitionNames.clone();
371376
}
372377
else {
373378
return StringUtils.toStringArray(this.beanDefinitionNames);
@@ -1266,8 +1271,9 @@ protected Map<String, Object> findAutowireCandidates(
12661271
addCandidateEntry(result, candidateName, descriptor, requiredType);
12671272
}
12681273
}
1269-
if (result.isEmpty()) {
1270-
// Consider self references before as a final pass
1274+
if (result.isEmpty() && !(descriptor instanceof MultiElementDependencyDescriptor)) {
1275+
// Consider self references as a final pass...
1276+
// but not as collection elements, just for direct dependency declarations.
12711277
for (String candidateName : candidateNames) {
12721278
if (isSelfReference(beanName, candidateName) && isAutowireCandidate(candidateName, fallbackDescriptor)) {
12731279
addCandidateEntry(result, candidateName, descriptor, requiredType);
@@ -1476,10 +1482,10 @@ private void checkBeanNotOfRequiredType(Class<?> type, DependencyDescriptor desc
14761482
Class<?> targetType = mbd.getTargetType();
14771483
if (targetType != null && type.isAssignableFrom(targetType) &&
14781484
isAutowireCandidate(beanName, mbd, descriptor, getAutowireCandidateResolver())) {
1479-
// Probably a poxy interfering with target type match -> throw meaningful exception.
1485+
// Probably a proxy interfering with target type match -> throw meaningful exception.
14801486
Object beanInstance = getSingleton(beanName, false);
14811487
Class<?> beanType = (beanInstance != null ? beanInstance.getClass() : predictBeanType(beanName, mbd));
1482-
if (type != beanType) {
1488+
if (!type.isAssignableFrom((beanType))) {
14831489
throw new BeanNotOfRequiredTypeException(beanName, type, beanType);
14841490
}
14851491
}

spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java

+1
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,7 @@ public void testNameAlreadyBound() {
776776
private void testSingleTestBean(ListableBeanFactory lbf) {
777777
assertTrue("1 beans defined", lbf.getBeanDefinitionCount() == 1);
778778
String[] names = lbf.getBeanDefinitionNames();
779+
assertTrue(names != lbf.getBeanDefinitionNames());
779780
assertTrue("Array length == 1", names.length == 1);
780781
assertTrue("0th element == test", names[0].equals("test"));
781782
TestBean tb = (TestBean) lbf.getBean("test");

spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java

+73-10
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.HashSet;
3030
import java.util.LinkedHashMap;
3131
import java.util.LinkedHashSet;
32+
import java.util.LinkedList;
3233
import java.util.List;
3334
import java.util.Map;
3435
import java.util.Properties;
@@ -920,7 +921,7 @@ public void testConstructorInjectionWithPlainMapAsBean() {
920921
RootBeanDefinition tbm = new RootBeanDefinition(CollectionFactoryMethods.class);
921922
tbm.setUniqueFactoryMethodName("testBeanMap");
922923
bf.registerBeanDefinition("myTestBeanMap", tbm);
923-
bf.registerSingleton("otherMap", new HashMap<Object, Object>());
924+
bf.registerSingleton("otherMap", new HashMap<>());
924925

925926
MapConstructorInjectionBean bean = (MapConstructorInjectionBean) bf.getBean("annotatedBean");
926927
assertSame(bf.getBean("myTestBeanMap"), bean.getTestBeanMap());
@@ -942,7 +943,7 @@ public void testConstructorInjectionWithTypedSetAsBean() {
942943
tbs.add(new TestBean("tb1"));
943944
tbs.add(new TestBean("tb2"));
944945
bf.registerSingleton("testBeans", tbs);
945-
bf.registerSingleton("otherSet", new HashSet<Object>());
946+
bf.registerSingleton("otherSet", new HashSet<>());
946947

947948
SetConstructorInjectionBean bean = (SetConstructorInjectionBean) bf.getBean("annotatedBean");
948949
assertSame(tbs, bean.getTestBeanSet());
@@ -963,7 +964,7 @@ public void testConstructorInjectionWithPlainSetAsBean() {
963964
RootBeanDefinition tbs = new RootBeanDefinition(CollectionFactoryMethods.class);
964965
tbs.setUniqueFactoryMethodName("testBeanSet");
965966
bf.registerBeanDefinition("myTestBeanSet", tbs);
966-
bf.registerSingleton("otherSet", new HashSet<Object>());
967+
bf.registerSingleton("otherSet", new HashSet<>());
967968

968969
SetConstructorInjectionBean bean = (SetConstructorInjectionBean) bf.getBean("annotatedBean");
969970
assertSame(bf.getBean("myTestBeanSet"), bean.getTestBeanSet());
@@ -978,11 +979,59 @@ public void testSelfReference() {
978979
AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
979980
bpp.setBeanFactory(bf);
980981
bf.addBeanPostProcessor(bpp);
981-
RootBeanDefinition bd = new RootBeanDefinition(SelfInjectionBean.class);
982-
bf.registerBeanDefinition("annotatedBean", bd);
982+
bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(SelfInjectionBean.class));
983+
984+
SelfInjectionBean bean = (SelfInjectionBean) bf.getBean("annotatedBean");
985+
assertSame(bean, bean.reference);
986+
assertNull(bean.referenceCollection);
987+
}
988+
989+
@Test
990+
public void testSelfReferenceWithOther() {
991+
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
992+
bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver());
993+
AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
994+
bpp.setBeanFactory(bf);
995+
bf.addBeanPostProcessor(bpp);
996+
bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(SelfInjectionBean.class));
997+
bf.registerBeanDefinition("annotatedBean2", new RootBeanDefinition(SelfInjectionBean.class));
983998

984999
SelfInjectionBean bean = (SelfInjectionBean) bf.getBean("annotatedBean");
985-
assertSame(bean, bean.selfReference);
1000+
SelfInjectionBean bean2 = (SelfInjectionBean) bf.getBean("annotatedBean2");
1001+
assertSame(bean2, bean.reference);
1002+
assertEquals(1, bean.referenceCollection.size());
1003+
assertSame(bean2, bean.referenceCollection.get(0));
1004+
}
1005+
1006+
@Test
1007+
public void testSelfReferenceCollection() {
1008+
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
1009+
bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver());
1010+
AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
1011+
bpp.setBeanFactory(bf);
1012+
bf.addBeanPostProcessor(bpp);
1013+
bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(SelfInjectionCollectionBean.class));
1014+
1015+
SelfInjectionCollectionBean bean = (SelfInjectionCollectionBean) bf.getBean("annotatedBean");
1016+
assertSame(bean, bean.reference);
1017+
assertNull(bean.referenceCollection);
1018+
}
1019+
1020+
@Test
1021+
public void testSelfReferenceCollectionWithOther() {
1022+
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
1023+
bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver());
1024+
AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
1025+
bpp.setBeanFactory(bf);
1026+
bf.addBeanPostProcessor(bpp);
1027+
bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(SelfInjectionCollectionBean.class));
1028+
bf.registerBeanDefinition("annotatedBean2", new RootBeanDefinition(SelfInjectionCollectionBean.class));
1029+
1030+
SelfInjectionCollectionBean bean = (SelfInjectionCollectionBean) bf.getBean("annotatedBean");
1031+
SelfInjectionCollectionBean bean2 = (SelfInjectionCollectionBean) bf.getBean("annotatedBean2");
1032+
assertSame(bean2, bean.reference);
1033+
assertSame(1, bean2.referenceCollection.size());
1034+
assertSame(bean2, bean.referenceCollection.get(0));
9861035
}
9871036

9881037
@Test
@@ -2582,7 +2631,21 @@ public Set<TestBean> getTestBeanSet() {
25822631
public static class SelfInjectionBean {
25832632

25842633
@Autowired
2585-
public SelfInjectionBean selfReference;
2634+
public SelfInjectionBean reference;
2635+
2636+
@Autowired(required = false)
2637+
public List<SelfInjectionBean> referenceCollection;
2638+
}
2639+
2640+
2641+
@SuppressWarnings("serial")
2642+
public static class SelfInjectionCollectionBean extends LinkedList<SelfInjectionCollectionBean> {
2643+
2644+
@Autowired
2645+
public SelfInjectionCollectionBean reference;
2646+
2647+
@Autowired(required = false)
2648+
public List<SelfInjectionCollectionBean> referenceCollection;
25862649
}
25872650

25882651

@@ -3149,7 +3212,7 @@ public static GenericInterface1<String> create() {
31493212
}
31503213

31513214
public static GenericInterface1<String> createErased() {
3152-
return new GenericInterface1Impl<String>();
3215+
return new GenericInterface1Impl<>();
31533216
}
31543217

31553218
@SuppressWarnings("rawtypes")
@@ -3336,14 +3399,14 @@ public ProvidedArgumentBean(String[] args) {
33363399
public static class CollectionFactoryMethods {
33373400

33383401
public static Map<String, TestBean> testBeanMap() {
3339-
Map<String, TestBean> tbm = new LinkedHashMap<String, TestBean>();
3402+
Map<String, TestBean> tbm = new LinkedHashMap<>();
33403403
tbm.put("testBean1", new TestBean("tb1"));
33413404
tbm.put("testBean2", new TestBean("tb2"));
33423405
return tbm;
33433406
}
33443407

33453408
public static Set<TestBean> testBeanSet() {
3346-
Set<TestBean> tbs = new LinkedHashSet<TestBean>();
3409+
Set<TestBean> tbs = new LinkedHashSet<>();
33473410
tbs.add(new TestBean("tb1"));
33483411
tbs.add(new TestBean("tb2"));
33493412
return tbs;

0 commit comments

Comments
 (0)