From 17dfc8b0fcf3e2a02ab0db48af1d309981bbe6c2 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 29 Jul 2009 16:43:12 +0000 Subject: [PATCH] fixed constructor resolution algorithm to trigger ambiguity exception as late as possible --- .../factory/support/ConstructorResolver.java | 42 +++++++++++----- .../XmlBeanFactoryTests-constructorArg.xml | 14 +++++- .../factory/xml/XmlBeanFactoryTests.java | 48 +++++++++++++++++-- 3 files changed, 85 insertions(+), 19 deletions(-) diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java index cfe37ae988..81e278ebca 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java @@ -153,6 +153,7 @@ class ConstructorResolver { } AutowireUtils.sortConstructors(candidates); int minTypeDiffWeight = Integer.MAX_VALUE; + Set ambiguousConstructors = null; for (int i = 0; i < candidates.length; i++) { Constructor candidate = candidates[i]; @@ -227,12 +228,14 @@ class ConstructorResolver { constructorToUse = candidate; argsToUse = args.arguments; minTypeDiffWeight = typeDiffWeight; + ambiguousConstructors = null; } - else if (typeDiffWeight < Integer.MAX_VALUE && typeDiffWeight == minTypeDiffWeight && - !mbd.isLenientConstructorResolution()) { - throw new BeanCreationException(mbd.getResourceDescription(), beanName, - "Ambiguous constructor matches found in bean '" + beanName + "' " + - "(hint: specify index/type/name arguments for simple parameters to avoid type ambiguities)"); + else if (constructorToUse != null && typeDiffWeight == minTypeDiffWeight) { + if (ambiguousConstructors == null) { + ambiguousConstructors = new LinkedHashSet(); + ambiguousConstructors.add(constructorToUse); + } + ambiguousConstructors.add(candidate); } } @@ -240,6 +243,12 @@ class ConstructorResolver { throw new BeanCreationException( mbd.getResourceDescription(), beanName, "Could not resolve matching constructor"); } + else if (ambiguousConstructors != null && !mbd.isLenientConstructorResolution()) { + throw new BeanCreationException(mbd.getResourceDescription(), beanName, + "Ambiguous constructor matches found in bean '" + beanName + "' " + + "(hint: specify index/type/name arguments for simple parameters to avoid type ambiguities): " + + ambiguousConstructors); + } if (explicitArgs == null) { mbd.resolvedConstructorOrFactoryMethod = constructorToUse; @@ -369,9 +378,10 @@ class ConstructorResolver { Method[] candidates = candidateSet.toArray(new Method[candidateSet.size()]); AutowireUtils.sortFactoryMethods(candidates); + ConstructorArgumentValues resolvedValues = null; boolean autowiring = (mbd.getResolvedAutowireMode() == RootBeanDefinition.AUTOWIRE_CONSTRUCTOR); int minTypeDiffWeight = Integer.MAX_VALUE; - ConstructorArgumentValues resolvedValues = null; + Set ambiguousFactoryMethods = null; int minNrOfArgs; if (explicitArgs != null) { @@ -444,12 +454,14 @@ class ConstructorResolver { factoryMethodToUse = candidate; argsToUse = args.arguments; minTypeDiffWeight = typeDiffWeight; + ambiguousFactoryMethods = null; } - else if (typeDiffWeight < Integer.MAX_VALUE && typeDiffWeight == minTypeDiffWeight && - !mbd.isLenientConstructorResolution()) { - throw new BeanCreationException(mbd.getResourceDescription(), beanName, - "Ambiguous factory method matches found in bean '" + beanName + "' " + - "(hint: specify index/type/name arguments for simple parameters to avoid type ambiguities)"); + else if (factoryMethodToUse != null && typeDiffWeight == minTypeDiffWeight) { + if ( ambiguousFactoryMethods == null) { + ambiguousFactoryMethods = new LinkedHashSet(); + ambiguousFactoryMethods.add(factoryMethodToUse); + } + ambiguousFactoryMethods.add(candidate); } } } @@ -461,11 +473,17 @@ class ConstructorResolver { "factory bean '" + mbd.getFactoryBeanName() + "'; " : "") + "factory method '" + mbd.getFactoryMethodName() + "'"); } - if (void.class.equals(factoryMethodToUse.getReturnType())) { + else if (void.class.equals(factoryMethodToUse.getReturnType())) { throw new BeanCreationException(mbd.getResourceDescription(), beanName, "Invalid factory method '" + mbd.getFactoryMethodName() + "': needs to have a non-void return type!"); } + else if (ambiguousFactoryMethods != null && !mbd.isLenientConstructorResolution()) { + throw new BeanCreationException(mbd.getResourceDescription(), beanName, + "Ambiguous factory method matches found in bean '" + beanName + "' " + + "(hint: specify index/type/name arguments for simple parameters to avoid type ambiguities): " + + ambiguousFactoryMethods); + } if (explicitArgs == null) { mbd.resolvedConstructorOrFactoryMethod = factoryMethodToUse; diff --git a/org.springframework.context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests-constructorArg.xml b/org.springframework.context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests-constructorArg.xml index af32261b91..905a4d6e01 100644 --- a/org.springframework.context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests-constructorArg.xml +++ b/org.springframework.context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests-constructorArg.xml @@ -196,7 +196,11 @@ test - + + + + + 1 @@ -204,7 +208,7 @@ - + 1 @@ -212,4 +216,10 @@ + + + + + + diff --git a/org.springframework.context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java b/org.springframework.context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java index 9f8e293bc0..2dcb6ee6e1 100644 --- a/org.springframework.context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java +++ b/org.springframework.context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java @@ -271,8 +271,8 @@ public final class XmlBeanFactoryTests { catch (BeanCreationException ex) { // Check whether message contains outer bean name. ex.printStackTrace(); - assertTrue(ex.getMessage().indexOf("failsOnInnerBean") != -1); - assertTrue(ex.getMessage().indexOf("someMap") != -1); + assertTrue(ex.getMessage().contains("failsOnInnerBean")); + assertTrue(ex.getMessage().contains("someMap")); } try { @@ -281,8 +281,8 @@ public final class XmlBeanFactoryTests { catch (BeanCreationException ex) { // Check whether message contains outer bean name. ex.printStackTrace(); - assertTrue(ex.getMessage().indexOf("failsOnInnerBeanForConstructor") != -1); - assertTrue(ex.getMessage().indexOf("constructor argument") != -1); + assertTrue(ex.getMessage().contains("failsOnInnerBeanForConstructor")); + assertTrue(ex.getMessage().contains("constructor argument")); } } @@ -1437,7 +1437,7 @@ public final class XmlBeanFactoryTests { } } - public @Test void testStringConstructor() { + public @Test void testJavaLangStringConstructor() { XmlBeanFactory xbf = new XmlBeanFactory(CONSTRUCTOR_ARG_CONTEXT); AbstractBeanDefinition bd = (AbstractBeanDefinition) xbf.getBeanDefinition("string"); bd.setLenientConstructorResolution(false); @@ -1445,6 +1445,14 @@ public final class XmlBeanFactoryTests { assertEquals("test", str); } + public @Test void testCustomStringConstructor() { + XmlBeanFactory xbf = new XmlBeanFactory(CONSTRUCTOR_ARG_CONTEXT); + AbstractBeanDefinition bd = (AbstractBeanDefinition) xbf.getBeanDefinition("stringConstructor"); + bd.setLenientConstructorResolution(false); + StringConstructorTestBean tb = (StringConstructorTestBean) xbf.getBean("stringConstructor"); + assertEquals("test", tb.name); + } + public @Test void testPrimitiveConstructorArray() { XmlBeanFactory xbf = new XmlBeanFactory(CONSTRUCTOR_ARG_CONTEXT); ConstructorArrayTestBean bean = (ConstructorArrayTestBean) xbf.getBean("constructorArray"); @@ -1461,6 +1469,22 @@ public final class XmlBeanFactoryTests { assertEquals(1, ((int[]) bean.array)[0]); } + public @Test void testStringConstructorArrayNoType() { + XmlBeanFactory xbf = new XmlBeanFactory(CONSTRUCTOR_ARG_CONTEXT); + ConstructorArrayTestBean bean = (ConstructorArrayTestBean) xbf.getBean("constructorArrayNoType"); + assertTrue(bean.array instanceof String[]); + assertEquals(0, ((String[]) bean.array).length); + } + + public @Test void testStringConstructorArrayNoTypeNonLenient() { + XmlBeanFactory xbf = new XmlBeanFactory(CONSTRUCTOR_ARG_CONTEXT); + AbstractBeanDefinition bd = (AbstractBeanDefinition) xbf.getBeanDefinition("constructorArrayNoType"); + bd.setLenientConstructorResolution(false); + ConstructorArrayTestBean bean = (ConstructorArrayTestBean) xbf.getBean("constructorArrayNoType"); + assertTrue(bean.array instanceof String[]); + assertEquals(0, ((String[]) bean.array).length); + } + public @Test void testWithDuplicateName() throws Exception { try { new XmlBeanFactory(TEST_WITH_DUP_NAMES_CONTEXT); @@ -1749,6 +1773,20 @@ public final class XmlBeanFactoryTests { public ConstructorArrayTestBean(short[] array) { this.array = array; } + + public ConstructorArrayTestBean(String[] array) { + this.array = array; + } + } + + + public static class StringConstructorTestBean { + + public final String name; + + public StringConstructorTestBean(String name) { + this.name = name; + } }