From a7849b2861eea7af18da9b0023bde5cfce2321da Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 24 Aug 2016 22:56:47 +0200 Subject: [PATCH] DefaultListableBeanFactory does not trigger early candidate creation ahead of primary bean selection Issue: SPR-14611 (cherry picked from commit c4fcdb6) --- .../support/DefaultListableBeanFactory.java | 182 +++++++++++------- .../DefaultListableBeanFactoryTests.java | 20 +- ...wiredAnnotationBeanPostProcessorTests.java | 5 +- .../core/annotation/OrderUtils.java | 8 +- 4 files changed, 124 insertions(+), 91 deletions(-) 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 724a9ab829..fad8a3956d 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 @@ -986,38 +986,47 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto return null; } + @SuppressWarnings("unchecked") private NamedBeanHolder resolveNamedBean(Class requiredType, Object... args) throws BeansException { Assert.notNull(requiredType, "Required type must not be null"); - String[] beanNames = getBeanNamesForType(requiredType); + String[] candidateNames = getBeanNamesForType(requiredType); - if (beanNames.length > 1) { - ArrayList autowireCandidates = new ArrayList(); - for (String beanName : beanNames) { + if (candidateNames.length > 1) { + List autowireCandidates = new ArrayList(candidateNames.length); + for (String beanName : candidateNames) { if (!containsBeanDefinition(beanName) || getBeanDefinition(beanName).isAutowireCandidate()) { autowireCandidates.add(beanName); } } if (!autowireCandidates.isEmpty()) { - beanNames = autowireCandidates.toArray(new String[autowireCandidates.size()]); + candidateNames = autowireCandidates.toArray(new String[autowireCandidates.size()]); } } - if (beanNames.length == 1) { - String beanName = beanNames[0]; + if (candidateNames.length == 1) { + String beanName = candidateNames[0]; return new NamedBeanHolder(beanName, getBean(beanName, requiredType, args)); } - else if (beanNames.length > 1) { - Map candidates = new LinkedHashMap(); - for (String beanName : beanNames) { - candidates.put(beanName, getBean(beanName, requiredType, args)); + else if (candidateNames.length > 1) { + Map candidates = new LinkedHashMap(candidateNames.length); + for (String candidateName : candidateNames) { + if (containsSingleton(candidateName)) { + candidates.put(candidateName, getBean(candidateName, requiredType, args)); + } + else { + candidates.put(candidateName, getType(candidateName)); + } } - String primaryCandidate = determinePrimaryCandidate(candidates, requiredType); - if (primaryCandidate != null) { - return new NamedBeanHolder(primaryCandidate, getBean(primaryCandidate, requiredType, args)); + String candidateName = determinePrimaryCandidate(candidates, requiredType); + if (candidateName == null) { + candidateName = determineHighestPriorityCandidate(candidates, requiredType); } - String priorityCandidate = determineHighestPriorityCandidate(candidates, requiredType); - if (priorityCandidate != null) { - return new NamedBeanHolder(priorityCandidate, getBean(priorityCandidate, requiredType, args)); + if (candidateName != null) { + Object beanInstance = candidates.get(candidateName); + if (beanInstance instanceof Class) { + beanInstance = getBean(candidateName, requiredType, args); + } + return new NamedBeanHolder(candidateName, (T) beanInstance); } throw new NoUniqueBeanDefinitionException(requiredType, candidates.keySet()); } @@ -1086,9 +1095,13 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } return null; } + + String autowiredBeanName; + Object instanceCandidate; + if (matchingBeans.size() > 1) { - String primaryBeanName = determineAutowireCandidate(matchingBeans, descriptor); - if (primaryBeanName == null) { + autowiredBeanName = determineAutowireCandidate(matchingBeans, descriptor); + if (autowiredBeanName == null) { if (descriptor.isRequired() || !indicatesMultipleBeans(type)) { return descriptor.resolveNotUnique(type, matchingBeans); } @@ -1099,17 +1112,20 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto return null; } } - if (autowiredBeanNames != null) { - autowiredBeanNames.add(primaryBeanName); - } - return matchingBeans.get(primaryBeanName); + instanceCandidate = matchingBeans.get(autowiredBeanName); } - // We have exactly one match. - Map.Entry entry = matchingBeans.entrySet().iterator().next(); + else { + // We have exactly one match. + Map.Entry entry = matchingBeans.entrySet().iterator().next(); + autowiredBeanName = entry.getKey(); + instanceCandidate = entry.getValue(); + } + if (autowiredBeanNames != null) { - autowiredBeanNames.add(entry.getKey()); + autowiredBeanNames.add(autowiredBeanName); } - return entry.getValue(); + return (instanceCandidate instanceof Class ? + descriptor.resolveCandidate(autowiredBeanName, type, this) : instanceCandidate); } finally { ConstructorResolver.setCurrentInjectionPoint(previousInjectionPoint); @@ -1122,9 +1138,8 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto Class type = descriptor.getDependencyType(); if (type.isArray()) { Class componentType = type.getComponentType(); - DependencyDescriptor targetDesc = new DependencyDescriptor(descriptor); - targetDesc.increaseNestingLevel(); - Map matchingBeans = findAutowireCandidates(beanName, componentType, targetDesc); + Map matchingBeans = findAutowireCandidates(beanName, componentType, + new MultiElementDependencyDescriptor(descriptor)); if (matchingBeans.isEmpty()) { return null; } @@ -1143,9 +1158,8 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto if (elementType == null) { return null; } - DependencyDescriptor targetDesc = new DependencyDescriptor(descriptor); - targetDesc.increaseNestingLevel(); - Map matchingBeans = findAutowireCandidates(beanName, elementType, targetDesc); + Map matchingBeans = findAutowireCandidates(beanName, elementType, + new MultiElementDependencyDescriptor(descriptor)); if (matchingBeans.isEmpty()) { return null; } @@ -1168,9 +1182,8 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto if (valueType == null) { return null; } - DependencyDescriptor targetDesc = new DependencyDescriptor(descriptor); - targetDesc.increaseNestingLevel(); - Map matchingBeans = findAutowireCandidates(beanName, valueType, targetDesc); + Map matchingBeans = findAutowireCandidates(beanName, valueType, + new MultiElementDependencyDescriptor(descriptor)); if (matchingBeans.isEmpty()) { return null; } @@ -1239,7 +1252,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } for (String candidateName : candidateNames) { if (!isSelfReference(beanName, candidateName) && isAutowireCandidate(candidateName, descriptor)) { - result.put(candidateName, descriptor.resolveCandidate(candidateName, requiredType, this)); + addCandidateEntry(result, candidateName, descriptor, requiredType); } } if (result.isEmpty() && !indicatesMultipleBeans(requiredType)) { @@ -1247,14 +1260,14 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto DependencyDescriptor fallbackDescriptor = descriptor.forFallbackMatch(); for (String candidateName : candidateNames) { if (!isSelfReference(beanName, candidateName) && isAutowireCandidate(candidateName, fallbackDescriptor)) { - result.put(candidateName, descriptor.resolveCandidate(candidateName, requiredType, this)); + addCandidateEntry(result, candidateName, descriptor, requiredType); } } if (result.isEmpty()) { // Consider self references before as a final pass for (String candidateName : candidateNames) { if (isSelfReference(beanName, candidateName) && isAutowireCandidate(candidateName, fallbackDescriptor)) { - result.put(candidateName, descriptor.resolveCandidate(candidateName, requiredType, this)); + addCandidateEntry(result, candidateName, descriptor, requiredType); } } } @@ -1262,31 +1275,46 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto return result; } + /** + * Add an entry to the candidate map: a bean instance if available or just the resolved + * type, preventing early bean initialization ahead of primary candidate selection. + */ + private void addCandidateEntry(Map candidates, String candidateName, + DependencyDescriptor descriptor, Class requiredType) { + + if (descriptor instanceof MultiElementDependencyDescriptor || containsSingleton(candidateName)) { + candidates.put(candidateName, descriptor.resolveCandidate(candidateName, requiredType, this)); + } + else { + candidates.put(candidateName, getType(candidateName)); + } + } + /** * Determine the autowire candidate in the given set of beans. *

Looks for {@code @Primary} and {@code @Priority} (in that order). - * @param candidateBeans a Map of candidate names and candidate instances + * @param candidates a Map of candidate names and candidate instances * that match the required type, as returned by {@link #findAutowireCandidates} * @param descriptor the target dependency to match against * @return the name of the autowire candidate, or {@code null} if none found */ - protected String determineAutowireCandidate(Map candidateBeans, DependencyDescriptor descriptor) { + protected String determineAutowireCandidate(Map candidates, DependencyDescriptor descriptor) { Class requiredType = descriptor.getDependencyType(); - String primaryCandidate = determinePrimaryCandidate(candidateBeans, requiredType); + String primaryCandidate = determinePrimaryCandidate(candidates, requiredType); if (primaryCandidate != null) { return primaryCandidate; } - String priorityCandidate = determineHighestPriorityCandidate(candidateBeans, requiredType); + String priorityCandidate = determineHighestPriorityCandidate(candidates, requiredType); if (priorityCandidate != null) { return priorityCandidate; } // Fallback - for (Map.Entry entry : candidateBeans.entrySet()) { - String candidateBeanName = entry.getKey(); + for (Map.Entry entry : candidates.entrySet()) { + String candidateName = entry.getKey(); Object beanInstance = entry.getValue(); if ((beanInstance != null && this.resolvableDependencies.containsValue(beanInstance)) || - matchesBeanName(candidateBeanName, descriptor.getDependencyName())) { - return candidateBeanName; + matchesBeanName(candidateName, descriptor.getDependencyName())) { + return candidateName; } } return null; @@ -1294,15 +1322,15 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto /** * Determine the primary candidate in the given set of beans. - * @param candidateBeans a Map of candidate names and candidate instances - * that match the required type + * @param candidates a Map of candidate names and candidate instances + * (or candidate classes if not created yet) that match the required type * @param requiredType the target dependency type to match against * @return the name of the primary candidate, or {@code null} if none found * @see #isPrimary(String, Object) */ - protected String determinePrimaryCandidate(Map candidateBeans, Class requiredType) { + protected String determinePrimaryCandidate(Map candidates, Class requiredType) { String primaryBeanName = null; - for (Map.Entry entry : candidateBeans.entrySet()) { + for (Map.Entry entry : candidates.entrySet()) { String candidateBeanName = entry.getKey(); Object beanInstance = entry.getValue(); if (isPrimary(candidateBeanName, beanInstance)) { @@ -1310,8 +1338,8 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto boolean candidateLocal = containsBeanDefinition(candidateBeanName); boolean primaryLocal = containsBeanDefinition(primaryBeanName); if (candidateLocal && primaryLocal) { - throw new NoUniqueBeanDefinitionException(requiredType, candidateBeans.size(), - "more than one 'primary' bean found among candidates: " + candidateBeans.keySet()); + throw new NoUniqueBeanDefinitionException(requiredType, candidates.size(), + "more than one 'primary' bean found among candidates: " + candidates.keySet()); } else if (candidateLocal) { primaryBeanName = candidateBeanName; @@ -1326,29 +1354,30 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } /** - * Determine the candidate with the highest priority in the given set of beans. As - * defined by the {@link org.springframework.core.Ordered} interface, the lowest - * value has the highest priority. - * @param candidateBeans a Map of candidate names and candidate instances - * that match the required type + * Determine the candidate with the highest priority in the given set of beans. + *

Based on {@code @javax.annotation.Priority}. As defined by the related + * {@link org.springframework.core.Ordered} interface, the lowest value has + * the highest priority. + * @param candidates a Map of candidate names and candidate instances + * (or candidate classes if not created yet) that match the required type * @param requiredType the target dependency type to match against * @return the name of the candidate with the highest priority, * or {@code null} if none found * @see #getPriority(Object) */ - protected String determineHighestPriorityCandidate(Map candidateBeans, Class requiredType) { + protected String determineHighestPriorityCandidate(Map candidates, Class requiredType) { String highestPriorityBeanName = null; Integer highestPriority = null; - for (Map.Entry entry : candidateBeans.entrySet()) { + for (Map.Entry entry : candidates.entrySet()) { String candidateBeanName = entry.getKey(); Object beanInstance = entry.getValue(); Integer candidatePriority = getPriority(beanInstance); if (candidatePriority != null) { if (highestPriorityBeanName != null) { if (candidatePriority.equals(highestPriority)) { - throw new NoUniqueBeanDefinitionException(requiredType, candidateBeans.size(), - "Multiple beans found with the same priority ('" + highestPriority + "') " + - "among candidates: " + candidateBeans.keySet()); + throw new NoUniqueBeanDefinitionException(requiredType, candidates.size(), + "Multiple beans found with the same priority ('" + highestPriority + + "') among candidates: " + candidates.keySet()); } else if (candidatePriority < highestPriority) { highestPriorityBeanName = candidateBeanName; @@ -1529,7 +1558,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto private class OptionalDependencyFactory { public Object createOptionalDependency(DependencyDescriptor descriptor, String beanName, final Object... args) { - DependencyDescriptor descriptorToUse = new DependencyDescriptor(descriptor) { + DependencyDescriptor descriptorToUse = new NestedDependencyDescriptor(descriptor) { @Override public boolean isRequired() { return false; @@ -1540,7 +1569,6 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto super.resolveCandidate(beanName, requiredType, beanFactory)); } }; - descriptorToUse.increaseNestingLevel(); return Optional.ofNullable(doResolveDependency(descriptorToUse, beanName, null, null)); } } @@ -1558,9 +1586,8 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto private final String beanName; public DependencyObjectProvider(DependencyDescriptor descriptor, String beanName) { - this.descriptor = new DependencyDescriptor(descriptor); - this.descriptor.increaseNestingLevel(); - this.optional = this.descriptor.getDependencyType().equals(javaUtilOptionalClass); + this.descriptor = new NestedDependencyDescriptor(descriptor); + this.optional = (this.descriptor.getDependencyType() == javaUtilOptionalClass); this.beanName = beanName; } @@ -1676,7 +1703,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto if (beanDefinition == null) { return null; } - List sources = new ArrayList(); + List sources = new ArrayList(2); Method factoryMethod = beanDefinition.getResolvedFactoryMethod(); if (factoryMethod != null) { sources.add(factoryMethod); @@ -1699,4 +1726,21 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } } + + private static class NestedDependencyDescriptor extends DependencyDescriptor { + + public NestedDependencyDescriptor(DependencyDescriptor original) { + super(original); + increaseNestingLevel(); + } + } + + + private static class MultiElementDependencyDescriptor extends NestedDependencyDescriptor { + + public MultiElementDependencyDescriptor(DependencyDescriptor original) { + super(original); + } + } + } 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 00e41497aa..71689a6a8a 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 @@ -1435,12 +1435,14 @@ public class DefaultListableBeanFactoryTests { public void testGetBeanByTypeWithPrimary() throws Exception { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); RootBeanDefinition bd1 = new RootBeanDefinition(TestBean.class); + bd1.setLazyInit(true); RootBeanDefinition bd2 = new RootBeanDefinition(TestBean.class); bd2.setPrimary(true); lbf.registerBeanDefinition("bd1", bd1); lbf.registerBeanDefinition("bd2", bd2); TestBean bean = lbf.getBean(TestBean.class); assertThat(bean.getBeanName(), equalTo("bd2")); + assertFalse(lbf.containsSingleton("bd1")); } @Test @@ -1760,24 +1762,6 @@ public class DefaultListableBeanFactoryTests { } } - @Test - public void testAutowireBeanByTypeWithTwoMatchesAndParameterNameDiscovery() { - DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); - RootBeanDefinition bd = new RootBeanDefinition(TestBean.class); - RootBeanDefinition bd2 = new RootBeanDefinition(TestBean.class); - lbf.registerBeanDefinition("test", bd); - lbf.registerBeanDefinition("spouse", bd2); - try { - lbf.autowire(DependenciesBean.class, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, true); - fail("Should have thrown UnsatisfiedDependencyException"); - } - catch (UnsatisfiedDependencyException ex) { - // expected - assertTrue(ex.getMessage().contains("test")); - assertTrue(ex.getMessage().contains("spouse")); - } - } - @Test public void testAutowireBeanByTypeWithDependencyCheck() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); 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 96e7e07a0f..21301e4cb1 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 @@ -1146,12 +1146,15 @@ public class AutowiredAnnotationBeanPostProcessorTests { RootBeanDefinition tb1 = new RootBeanDefinition(TestBean.class); tb1.setPrimary(true); bf.registerBeanDefinition("testBean1", tb1); - bf.registerBeanDefinition("testBean2", new RootBeanDefinition(TestBean.class)); + RootBeanDefinition tb2 = new RootBeanDefinition(TestBean.class); + tb2.setLazyInit(true); + bf.registerBeanDefinition("testBean2", tb2); SmartObjectFactoryInjectionBean bean = (SmartObjectFactoryInjectionBean) bf.getBean("annotatedBean"); assertSame(bf.getBean("testBean1"), bean.getTestBean()); assertSame(bf.getBean("testBean1"), bean.getOptionalTestBean()); assertSame(bf.getBean("testBean1"), bean.getUniqueTestBean()); + assertFalse(bf.containsSingleton("testBean2")); bf.destroySingletons(); } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/OrderUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/OrderUtils.java index 350a7bdf6f..5b2cf361fe 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/OrderUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/OrderUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 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. @@ -48,9 +48,10 @@ public abstract class OrderUtils { /** * Return the order on the specified {@code type}. - *

Take care of {@link Order @Order} and {@code @javax.annotation.Priority}. + *

Takes care of {@link Order @Order} and {@code @javax.annotation.Priority}. * @param type the type to handle * @return the order value, or {@code null} if none can be found + * @see #getPriority(Class) */ public static Integer getOrder(Class type) { return getOrder(type, null); @@ -59,9 +60,10 @@ public abstract class OrderUtils { /** * Return the order on the specified {@code type}, or the specified * default value if none can be found. - *

Take care of {@link Order @Order} and {@code @javax.annotation.Priority}. + *

Takes care of {@link Order @Order} and {@code @javax.annotation.Priority}. * @param type the type to handle * @return the priority value, or the specified default order if none can be found + * @see #getPriority(Class) */ public static Integer getOrder(Class type, Integer defaultOrder) { Order order = AnnotationUtils.findAnnotation(type, Order.class);