diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java index 384866ffdd78..f290d51b1869 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -151,7 +151,7 @@ public BeanWrapper autowireConstructor( // Take specified constructors, if any. Constructor[] candidates = chosenCtors; if (candidates == null) { - Class beanClass = mbd.getBeanClass(); + Class beanClass = mbd.getBeanClass(); try { candidates = (mbd.isNonPublicAccessAllowed() ? beanClass.getDeclaredConstructors() : beanClass.getConstructors()); @@ -169,7 +169,7 @@ public BeanWrapper autowireConstructor( for (int i = 0; i < candidates.length; i++) { Constructor candidate = candidates[i]; - Class[] paramTypes = candidate.getParameterTypes(); + Class[] paramTypes = candidate.getParameterTypes(); if (constructorToUse != null && argsToUse.length > paramTypes.length) { // Already found greedy constructor that can be satisfied -> @@ -341,7 +341,7 @@ public BeanWrapper instantiateUsingFactoryMethod(final String beanName, final Ro this.beanFactory.initBeanWrapper(bw); Object factoryBean; - Class factoryClass; + Class factoryClass; boolean isStatic; String factoryBeanName = mbd.getFactoryBeanName(); @@ -399,7 +399,7 @@ public BeanWrapper instantiateUsingFactoryMethod(final String beanName, final Ro factoryClass = ClassUtils.getUserClass(factoryClass); Method[] rawCandidates; - final Class factoryClazz = factoryClass; + final Class factoryClazz = factoryClass; if (System.getSecurityManager() != null) { rawCandidates = AccessController.doPrivileged(new PrivilegedAction() { public Method[] run() { @@ -445,7 +445,7 @@ public Method[] run() { for (int i = 0; i < candidates.length; i++) { Method candidate = candidates[i]; - Class[] paramTypes = candidate.getParameterTypes(); + Class[] paramTypes = candidate.getParameterTypes(); if (paramTypes.length >= minNrOfArgs) { ArgumentsHolder argsHolder; @@ -503,7 +503,15 @@ public Method[] run() { minTypeDiffWeight = typeDiffWeight; ambiguousFactoryMethods = null; } - else if (factoryMethodToUse != null && typeDiffWeight == minTypeDiffWeight) { + // Find out about ambiguity: In case of the same type difference weight + // for methods with the same number of parameters, collect such candidates + // and eventually raise an ambiguity exception. + // However, only perform that check in non-lenient constructor resolution mode, + // and explicitly ignore overridden methods (with the same parameter signature). + else if (factoryMethodToUse != null && typeDiffWeight == minTypeDiffWeight && + !mbd.isLenientConstructorResolution() && + paramTypes.length == factoryMethodToUse.getParameterTypes().length && + !Arrays.equals(paramTypes, factoryMethodToUse.getParameterTypes())) { if (ambiguousFactoryMethods == null) { ambiguousFactoryMethods = new LinkedHashSet(); ambiguousFactoryMethods.add(factoryMethodToUse); @@ -540,7 +548,7 @@ else if (void.class.equals(factoryMethodToUse.getReturnType())) { "Invalid factory method '" + mbd.getFactoryMethodName() + "': needs to have a non-void return type!"); } - else if (ambiguousFactoryMethods != null && !mbd.isLenientConstructorResolution()) { + else if (ambiguousFactoryMethods != null) { 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): " + @@ -644,7 +652,7 @@ private int resolveConstructorArguments( */ private ArgumentsHolder createArgumentArray( String beanName, RootBeanDefinition mbd, ConstructorArgumentValues resolvedValues, - BeanWrapper bw, Class[] paramTypes, String[] paramNames, Object methodOrCtor, + BeanWrapper bw, Class[] paramTypes, String[] paramNames, Object methodOrCtor, boolean autowiring) throws UnsatisfiedDependencyException { String methodType = (methodOrCtor instanceof Constructor ? "constructor" : "factory method"); @@ -750,7 +758,7 @@ private ArgumentsHolder createArgumentArray( private Object[] resolvePreparedArguments( String beanName, RootBeanDefinition mbd, BeanWrapper bw, Member methodOrCtor, Object[] argsToResolve) { - Class[] paramTypes = (methodOrCtor instanceof Method ? + Class[] paramTypes = (methodOrCtor instanceof Method ? ((Method) methodOrCtor).getParameterTypes() : ((Constructor) methodOrCtor).getParameterTypes()); TypeConverter converter = (this.beanFactory.getCustomTypeConverter() != null ? this.beanFactory.getCustomTypeConverter() : bw); @@ -822,7 +830,7 @@ public ArgumentsHolder(Object[] args) { this.preparedArguments = args; } - public int getTypeDifferenceWeight(Class[] paramTypes) { + public int getTypeDifferenceWeight(Class[] paramTypes) { // If valid arguments found, determine type difference weight. // Try type difference weight on both the converted arguments and // the raw arguments. If the raw weight is better, use it. @@ -832,7 +840,7 @@ public int getTypeDifferenceWeight(Class[] paramTypes) { return (rawTypeDiffWeight < typeDiffWeight ? rawTypeDiffWeight : typeDiffWeight); } - public int getAssignabilityWeight(Class[] paramTypes) { + public int getAssignabilityWeight(Class[] paramTypes) { for (int i = 0; i < paramTypes.length; i++) { if (!ClassUtils.isAssignableValue(paramTypes[i], this.arguments[i])) { return Integer.MAX_VALUE; diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java index 27ae171955cc..c5b07237cf19 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java @@ -310,6 +310,7 @@ private static class ConfigurationClassBeanDefinition extends RootBeanDefinition public ConfigurationClassBeanDefinition(ConfigurationClass configClass) { this.annotationMetadata = configClass.getMetadata(); + setLenientConstructorResolution(false); } public ConfigurationClassBeanDefinition(RootBeanDefinition original, ConfigurationClass configClass) { diff --git a/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java b/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java index 1d573540f8a2..b666f8be9229 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java @@ -16,18 +16,20 @@ package org.springframework.context.annotation; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.lang.annotation.Inherited; +import java.util.List; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; + import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.RootBeanDefinition; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + /** * Tests regarding overloading and overriding of bean methods. * Related to SPR-6618. @@ -42,22 +44,27 @@ * the most specific subclass bean method will always be the one that is invoked. * * @author Chris Beams + * @author Phillip Webb */ +@SuppressWarnings("resource") public class BeanMethodPolymorphismTests { + @Rule + public ExpectedException thrown = ExpectedException.none(); + + @Test public void beanMethodOverloadingWithoutInheritance() { + @SuppressWarnings({ "hiding" }) @Configuration class Config { @Bean String aString() { return "na"; } @Bean String aString(Integer dependency) { return "na"; } } - try { - new AnnotationConfigApplicationContext(Config.class); - fail("expected bean method overloading exception"); - } catch (BeanDefinitionParsingException ex) { - assertTrue(ex.getMessage(), ex.getMessage().contains("2 overloaded @Bean methods named 'aString'")); - } + + this.thrown.expect(BeanDefinitionParsingException.class); + this.thrown.expectMessage("overloaded @Bean methods named 'aString'"); + new AnnotationConfigApplicationContext(Config.class); } @Test @@ -65,12 +72,12 @@ public void beanMethodOverloadingWithInheritance() { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfig.class); assertThat(ctx.getBean(String.class), equalTo("overloaded5")); } - static @Configuration class SuperConfig { - @Bean String aString() { return "super"; } - } - static @Configuration class SubConfig { - @Bean Integer anInt() { return 5; } - @Bean String aString(Integer dependency) { return "overloaded"+dependency; } + + @Test + public void beanMethodOverloadingWithInheritanceAndList() { + // SPR-11025 + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfigWithList.class); + assertThat(ctx.getBean(String.class), equalTo("overloaded5")); } /** @@ -83,10 +90,6 @@ public void beanMethodShadowing() { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ShadowConfig.class); assertThat(ctx.getBean(String.class), equalTo("shadow")); } - @Import(SubConfig.class) - static @Configuration class ShadowConfig { - @Bean String aString() { return "shadow"; } - } /** * Tests that polymorphic Configuration classes need not explicitly redeclare the @@ -110,6 +113,7 @@ static class BaseConfig { public TestBean testBean() { return new TestBean(); } + } @@ -117,4 +121,55 @@ public TestBean testBean() { static class Config extends BaseConfig { } + + @Configuration + static class SuperConfig { + + @Bean + String aString() { + return "super"; + } + } + + + @Configuration + static class SubConfig extends SuperConfig { + + @Bean + Integer anInt() { + return 5; + } + + @Bean + String aString(Integer dependency) { + return "overloaded" + dependency; + } + } + + + @Configuration + static class SubConfigWithList extends SuperConfig { + + @Bean + Integer anInt() { + return 5; + } + + @Bean + String aString(List dependency) { + return "overloaded" + dependency.get(0); + } + } + + + @Configuration + @Import(SubConfig.class) + static class ShadowConfig { + + @Bean + String aString() { + return "shadow"; + } + } + } diff --git a/spring-core/src/main/java/org/springframework/util/MethodInvoker.java b/spring-core/src/main/java/org/springframework/util/MethodInvoker.java index 565a68c696dd..b1bd04ccb380 100644 --- a/spring-core/src/main/java/org/springframework/util/MethodInvoker.java +++ b/spring-core/src/main/java/org/springframework/util/MethodInvoker.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -40,7 +40,7 @@ */ public class MethodInvoker { - private Class targetClass; + private Class targetClass; private Object targetObject; @@ -61,14 +61,14 @@ public class MethodInvoker { * @see #setTargetObject * @see #setTargetMethod */ - public void setTargetClass(Class targetClass) { + public void setTargetClass(Class targetClass) { this.targetClass = targetClass; } /** * Return the target class on which to call the target method. */ - public Class getTargetClass() { + public Class getTargetClass() { return this.targetClass; } @@ -158,7 +158,7 @@ public void prepare() throws ClassNotFoundException, NoSuchMethodException { this.targetMethod = methodName; } - Class targetClass = getTargetClass(); + Class targetClass = getTargetClass(); String targetMethod = getTargetMethod(); if (targetClass == null) { throw new IllegalArgumentException("Either 'targetClass' or 'targetObject' is required"); @@ -194,7 +194,7 @@ public void prepare() throws ClassNotFoundException, NoSuchMethodException { * @return the resolved Class * @throws ClassNotFoundException if the class name was invalid */ - protected Class resolveClassName(String className) throws ClassNotFoundException { + protected Class resolveClassName(String className) throws ClassNotFoundException { return ClassUtils.forName(className, ClassUtils.getDefaultClassLoader()); } @@ -287,19 +287,22 @@ public Object invoke() throws InvocationTargetException, IllegalAccessException * Therefore, with an arg of type Integer, a constructor (Integer) would be preferred to a * constructor (Number) which would in turn be preferred to a constructor (Object). * All argument weights get accumulated. + *

Note: This is the algorithm used by MethodInvoker itself and also the algorithm + * used for constructor and factory method selection in Spring's bean container (in case + * of lenient constructor resolution which is the default for regular bean definitions). * @param paramTypes the parameter types to match * @param args the arguments to match * @return the accumulated weight for all arguments */ - public static int getTypeDifferenceWeight(Class[] paramTypes, Object[] args) { + public static int getTypeDifferenceWeight(Class[] paramTypes, Object[] args) { int result = 0; for (int i = 0; i < paramTypes.length; i++) { if (!ClassUtils.isAssignableValue(paramTypes[i], args[i])) { return Integer.MAX_VALUE; } if (args[i] != null) { - Class paramType = paramTypes[i]; - Class superClass = args[i].getClass().getSuperclass(); + Class paramType = paramTypes[i]; + Class superClass = args[i].getClass().getSuperclass(); while (superClass != null) { if (paramType.equals(superClass)) { result = result + 2;