From c9d08bff4199adb42f138076cc91a3990c0515c4 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 16 Feb 2018 17:27:42 +0100 Subject: [PATCH] DefaultListableBeanFactory only calls getPriority for non-null instance Issue: SPR-16508 --- .../support/DefaultListableBeanFactory.java | 35 +++++----- .../DefaultListableBeanFactoryTests.java | 66 ++++++++++++++----- 2 files changed, 67 insertions(+), 34 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 244de92c56..7bddd96f38 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 @@ -1006,7 +1006,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } } if (!autowireCandidates.isEmpty()) { - candidateNames = autowireCandidates.toArray(new String[autowireCandidates.size()]); + candidateNames = StringUtils.toStringArray(autowireCandidates); } } @@ -1017,8 +1017,9 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto else if (candidateNames.length > 1) { Map candidates = new LinkedHashMap<>(candidateNames.length); for (String beanName : candidateNames) { - if (containsSingleton(beanName)) { - candidates.put(beanName, getBean(beanName, requiredType, args)); + if (containsSingleton(beanName) && args == null) { + Object beanInstance = getBean(beanName); + candidates.put(beanName, (beanInstance instanceof NullBean ? null : beanInstance)); } else { candidates.put(beanName, getType(beanName)); @@ -1030,7 +1031,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } if (candidateName != null) { Object beanInstance = candidates.get(candidateName); - if (beanInstance instanceof Class) { + if (beanInstance == null || beanInstance instanceof Class) { beanInstance = getBean(candidateName, requiredType, args); } return new NamedBeanHolder<>(candidateName, (T) beanInstance); @@ -1413,23 +1414,25 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto 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, candidates.size(), - "Multiple beans found with the same priority ('" + highestPriority + - "') among candidates: " + candidates.keySet()); + if (beanInstance != null) { + Integer candidatePriority = getPriority(beanInstance); + if (candidatePriority != null) { + if (highestPriorityBeanName != null) { + if (candidatePriority.equals(highestPriority)) { + throw new NoUniqueBeanDefinitionException(requiredType, candidates.size(), + "Multiple beans found with the same priority ('" + highestPriority + + "') among candidates: " + candidates.keySet()); + } + else if (candidatePriority < highestPriority) { + highestPriorityBeanName = candidateBeanName; + highestPriority = candidatePriority; + } } - else if (candidatePriority < highestPriority) { + else { highestPriorityBeanName = candidateBeanName; highestPriority = candidatePriority; } } - else { - highestPriorityBeanName = candidateBeanName; - highestPriority = candidatePriority; - } } } return highestPriorityBeanName; 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 7c5390ee00..4042868443 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 @@ -1434,7 +1434,7 @@ public class DefaultListableBeanFactoryTests { } @Test - public void testGetBeanByTypeWithPrimary() throws Exception { + public void testGetBeanByTypeWithPrimary() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); RootBeanDefinition bd1 = new RootBeanDefinition(TestBean.class); bd1.setLazyInit(true); @@ -1448,7 +1448,7 @@ public class DefaultListableBeanFactoryTests { } @Test - public void testGetBeanByTypeWithMultiplePrimary() throws Exception { + public void testGetBeanByTypeWithMultiplePrimary() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); RootBeanDefinition bd1 = new RootBeanDefinition(TestBean.class); bd1.setPrimary(true); @@ -1462,19 +1462,39 @@ public class DefaultListableBeanFactoryTests { } @Test - public void testGetBeanByTypeWithPriority() throws Exception { + public void testGetBeanByTypeWithPriority() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); lbf.setDependencyComparator(AnnotationAwareOrderComparator.INSTANCE); RootBeanDefinition bd1 = new RootBeanDefinition(HighPriorityTestBean.class); RootBeanDefinition bd2 = new RootBeanDefinition(LowPriorityTestBean.class); + RootBeanDefinition bd3 = new RootBeanDefinition(NullTestBeanFactoryBean.class); lbf.registerBeanDefinition("bd1", bd1); lbf.registerBeanDefinition("bd2", bd2); + lbf.registerBeanDefinition("bd3", bd3); + lbf.preInstantiateSingletons(); TestBean bean = lbf.getBean(TestBean.class); assertThat(bean.getBeanName(), equalTo("bd1")); } @Test - public void testGetBeanByTypeWithMultiplePriority() throws Exception { + public void testMapInjectionWithPriority() { + DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); + lbf.setDependencyComparator(AnnotationAwareOrderComparator.INSTANCE); + RootBeanDefinition bd1 = new RootBeanDefinition(HighPriorityTestBean.class); + RootBeanDefinition bd2 = new RootBeanDefinition(LowPriorityTestBean.class); + RootBeanDefinition bd3 = new RootBeanDefinition(NullTestBeanFactoryBean.class); + RootBeanDefinition bd4 = new RootBeanDefinition(TestBeanRecipient.class, RootBeanDefinition.AUTOWIRE_CONSTRUCTOR, false); + lbf.registerBeanDefinition("bd1", bd1); + lbf.registerBeanDefinition("bd2", bd2); + lbf.registerBeanDefinition("bd3", bd3); + lbf.registerBeanDefinition("bd4", bd4); + lbf.preInstantiateSingletons(); + TestBean bean = lbf.getBean(TestBeanRecipient.class).testBean; + assertThat(bean.getBeanName(), equalTo("bd1")); + } + + @Test + public void testGetBeanByTypeWithMultiplePriority() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); lbf.setDependencyComparator(AnnotationAwareOrderComparator.INSTANCE); RootBeanDefinition bd1 = new RootBeanDefinition(HighPriorityTestBean.class); @@ -1483,12 +1503,12 @@ public class DefaultListableBeanFactoryTests { lbf.registerBeanDefinition("bd2", bd2); thrown.expect(NoUniqueBeanDefinitionException.class); thrown.expectMessage(containsString("Multiple beans found with the same priority")); - thrown.expectMessage(containsString("5")); // conflicting priority + thrown.expectMessage(containsString("5")); // conflicting priority lbf.getBean(TestBean.class); } @Test - public void testGetBeanByTypeWithPriorityAndNullInstance() throws Exception { + public void testGetBeanByTypeWithPriorityAndNullInstance() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); lbf.setDependencyComparator(AnnotationAwareOrderComparator.INSTANCE); RootBeanDefinition bd1 = new RootBeanDefinition(HighPriorityTestBean.class); @@ -1500,7 +1520,7 @@ public class DefaultListableBeanFactoryTests { } @Test - public void testGetBeanByTypePrimaryHasPrecedenceOverPriority() throws Exception { + public void testGetBeanByTypePrimaryHasPrecedenceOverPriority() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); lbf.setDependencyComparator(AnnotationAwareOrderComparator.INSTANCE); RootBeanDefinition bd1 = new RootBeanDefinition(HighPriorityTestBean.class); @@ -1522,7 +1542,7 @@ public class DefaultListableBeanFactoryTests { lbf.registerBeanDefinition("bd1", bd1); lbf.registerBeanDefinition("na1", na1); - TestBean actual = lbf.getBean(TestBean.class); // na1 was filtered + TestBean actual = lbf.getBean(TestBean.class); // na1 was filtered assertSame(lbf.getBean("bd1", TestBean.class), actual); lbf.registerBeanDefinition("bd2", bd2); @@ -1605,7 +1625,7 @@ public class DefaultListableBeanFactoryTests { lbf.registerBeanDefinition("bd1", bd1); lbf.registerBeanDefinition("na1", na1); - ConstructorDependency actual = lbf.getBean(ConstructorDependency.class, 42); // na1 was filtered + ConstructorDependency actual = lbf.getBean(ConstructorDependency.class, 42); // na1 was filtered assertThat(actual.beanName, equalTo("bd1")); lbf.registerBeanDefinition("bd2", bd2); @@ -1850,7 +1870,7 @@ public class DefaultListableBeanFactoryTests { // expected assertNotNull("Exception should have cause", ex.getCause()); assertEquals("Wrong cause type", NoUniqueBeanDefinitionException.class, ex.getCause().getClass()); - assertTrue(ex.getMessage().contains("5")); // conflicting priority + assertTrue(ex.getMessage().contains("5")); // conflicting priority } } @@ -2278,7 +2298,7 @@ public class DefaultListableBeanFactoryTests { /** * @Test - * public void testPrototypeCreationIsFastEnough2() throws Exception { + * public void testPrototypeCreationIsFastEnough2() { * if (factoryLog.isTraceEnabled() || factoryLog.isDebugEnabled()) { * // Skip this test: Trace logging blows the time limit. * return; @@ -2576,7 +2596,7 @@ public class DefaultListableBeanFactoryTests { } @Test(expected = IllegalStateException.class) - public void testScopingBeanToUnregisteredScopeResultsInAnException() throws Exception { + public void testScopingBeanToUnregisteredScopeResultsInAnException() { BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(TestBean.class); AbstractBeanDefinition beanDefinition = builder.getBeanDefinition(); beanDefinition.setScope("he put himself so low could hardly look me in the face"); @@ -2587,7 +2607,7 @@ public class DefaultListableBeanFactoryTests { } @Test - public void testExplicitScopeInheritanceForChildBeanDefinitions() throws Exception { + public void testExplicitScopeInheritanceForChildBeanDefinitions() { String theChildScope = "bonanza!"; RootBeanDefinition parent = new RootBeanDefinition(); @@ -2606,7 +2626,7 @@ public class DefaultListableBeanFactoryTests { } @Test - public void testScopeInheritanceForChildBeanDefinitions() throws Exception { + public void testScopeInheritanceForChildBeanDefinitions() { RootBeanDefinition parent = new RootBeanDefinition(); parent.setScope("bonanza!"); @@ -2993,7 +3013,7 @@ public class DefaultListableBeanFactoryTests { } @Override - public T call() throws Exception { + public T call() { throw new IllegalStateException(); } } @@ -3004,7 +3024,7 @@ public class DefaultListableBeanFactoryTests { public boolean initialized = false; @Override - public Object getObject() throws Exception { + public Object getObject() { this.initialized = true; return ""; } @@ -3026,7 +3046,7 @@ public class DefaultListableBeanFactoryTests { public boolean initialized = false; @Override - public Object getObject() throws Exception { + public Object getObject() { this.initialized = true; return ""; } @@ -3258,7 +3278,7 @@ public class DefaultListableBeanFactoryTests { private static class NullTestBeanFactoryBean implements FactoryBean { @Override - public TestBean getObject() throws Exception { + public TestBean getObject() { return null; } @@ -3274,6 +3294,16 @@ public class DefaultListableBeanFactoryTests { } + private static class TestBeanRecipient { + + public TestBean testBean; + + public TestBeanRecipient(TestBean testBean) { + this.testBean = testBean; + } + } + + enum NonPublicEnum { VALUE_1, VALUE_2;