Introduce ReflectionUtils#getUniqueDeclaredMethods

This change is in support of certain polymorphism cases in
@Configuration class inheritance hierarchies.  Consider the following
scenario:

@Configuration
public abstract class AbstractConfig {
    public abstract Object bean();
}

@Configuration
public class ConcreteConfig {
    @Override
    @Bean
    public BeanPostProcessor bean() { ... }
}

ConcreteConfig overrides AbstractConfig's #bean() method with a
covariant return type, in this case returning an object of type
BeanPostProcessor.  It is critically important that the container
is able to detect the return type of ConcreteConfig#bean() in order
to instantiate the BPP at the right point in the lifecycle.

Prior to this change, the container could not do this.
AbstractAutowireCapableBeanFactory#getTypeForFactoryMethod called
ReflectionUtils#getAllDeclaredMethods, which returned Method objects
for both the Object and BeanPostProcessor signatures of the #bean()
method.  This confused the implementation sufficiently as not to
choose a type for the factory method at all.  This means that the
BPP never gets detected as a BPP.

The new method being introduced here, #getUniqueDeclaredMethods, takes
covariant return types into account, and filters out duplicates,
favoring the most specific / narrow return type.

Additionally, it filters out any CGLIB 'rewritten' methods, which
is important in the case of @Configuration classes, which are
enhanced by CGLIB.  See the implementation for further details.

org.springframework.aop/src/main/java/org/springframework/aop/support/AopUtils.java

@@ -23,6 +23,8 @@ import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.springframework.aop.Advisor;
 import org.springframework.aop.AopInvocationException;

org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

@@ -631,7 +631,7 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 		// If all factory methods have the same return type, return that type.
 		// Can't clearly figure out exact method due to type converting / autowiring!
 		int minNrOfArgs = mbd.getConstructorArgumentValues().getArgumentCount();
-		Method[] candidates = ReflectionUtils.getAllDeclaredMethods(factoryClass);
+		Method[] candidates = ReflectionUtils.getUniqueDeclaredMethods(factoryClass);
 		Set<Class> returnTypes = new HashSet<Class>(1);
 		for (Method factoryMethod : candidates) {
 			if (Modifier.isStatic(factoryMethod.getModifiers()) == isStatic &&

org.springframework.context/src/test/java/org/springframework/context/annotation/ReflectionUtilsIntegrationTests.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright 2002-2011 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.context.annotation;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import java.lang.reflect.Method;
+
+import org.junit.Test;
+import org.springframework.beans.factory.support.DefaultListableBeanFactory;
+import org.springframework.util.ReflectionUtils;
+
+/**
+ * Tests ReflectionUtils methods as used against CGLIB-generated classes created
+ * by ConfigurationClassEnhancer.
+ *
+ * @author Chris Beams
+ * @since 3.1
+ * @see org.springframework.util.ReflectionUtilsTests
+ */
+public class ReflectionUtilsIntegrationTests {
+
+	@Test
+	public void getUniqueDeclaredMethods_withCovariantReturnType_andCglibRewrittenMethodNames() throws Exception {
+		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
+		Class<?> cglibLeaf = new ConfigurationClassEnhancer(bf).enhance(Leaf.class);
+		int m1MethodCount = 0;
+		Method[] methods = ReflectionUtils.getUniqueDeclaredMethods(cglibLeaf);
+		for (Method method : methods) {
+			if (method.getName().equals("m1")) {
+				m1MethodCount++;
+			}
+		}
+		assertThat(m1MethodCount, is(1));
+		for (Method method : methods) {
+			if (method.getName().contains("m1")) {
+				assertEquals(method.getReturnType(), Integer.class);
+			}
+		}
+	}
+
+
+	@Configuration
+	static abstract class Parent {
+		public abstract Number m1();
+	}
+
+
+	@Configuration
+	static class Leaf extends Parent {
+		@Override
+		@Bean
+		public Integer m1() {
+			return new Integer(42);
+		}
+	}
+
+}

org.springframework.core/src/main/java/org/springframework/util/ReflectionUtils.java

@@ -26,6 +26,7 @@ import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.regex.Pattern;
 
 /**
  * Simple utility class for working with the reflection API and handling
@@ -38,10 +39,13 @@ import java.util.List;
  * @author Rod Johnson
  * @author Costin Leau
  * @author Sam Brannen
+ * @author Chris Beams
  * @since 1.2.2
  */
 public abstract class ReflectionUtils {
 
+	private static final Pattern CGLIB_RENAMED_METHOD_PATTERN = Pattern.compile("CGLIB\\$(.+)\\$\\d+");
+
 	/**
 	 * Attempt to find a {@link Field field} on the supplied {@link Class} with the
 	 * supplied <code>name</code>. Searches all superclasses up to {@link Object}.
@@ -379,6 +383,16 @@ public abstract class ReflectionUtils {
 		}
 	}
 
+	/**
+	 * Determine whether the given method is a CGLIB 'renamed' method, following
+	 * the pattern "CGLIB$methodName$0".
+	 * @param renamedMethod the method to check
+	 * @see net.sf.cglib.proxy.Enhancer#rename
+	 */
+	public static boolean isCglibRenamedMethod(Method renamedMethod) {
+		return CGLIB_RENAMED_METHOD_PATTERN.matcher(renamedMethod.getName()).matches();
+	}
+
 	/**
 	 * Make the given field accessible, explicitly setting it accessible if
 	 * necessary. The <code>setAccessible(true)</code> method is only called
@@ -487,6 +501,42 @@ public abstract class ReflectionUtils {
 		return methods.toArray(new Method[methods.size()]);
 	}
 
+	/**
+	 * Get the unique set of declared methods on the leaf class and all superclasses. Leaf
+	 * class methods are included first and while traversing the superclass hierarchy any methods found
+	 * with signatures matching a method already included are filtered out.
+	 */
+	public static Method[] getUniqueDeclaredMethods(Class<?> leafClass) throws IllegalArgumentException {
+		final List<Method> methods = new ArrayList<Method>(32);
+		doWithMethods(leafClass, new MethodCallback() {
+			public void doWith(Method method) {
+				boolean knownSignature = false;
+				Method methodBeingOverriddenWithCovariantReturnType = null;
+
+				for (Method existingMethod : methods) {
+					if (method.getName().equals(existingMethod.getName()) &&
+							Arrays.equals(method.getParameterTypes(), existingMethod.getParameterTypes())) {
+						// is this a covariant return type situation?
+						if (existingMethod.getReturnType() != method.getReturnType() &&
+								existingMethod.getReturnType().isAssignableFrom(method.getReturnType())) {
+							methodBeingOverriddenWithCovariantReturnType = existingMethod;
+						} else {
+							knownSignature = true;
+						}
+						break;
+					}
+				}
+				if (methodBeingOverriddenWithCovariantReturnType != null) {
+					methods.remove(methodBeingOverriddenWithCovariantReturnType);
+				}
+				if (!knownSignature && !isCglibRenamedMethod(method)) {
+					methods.add(method);
+				}
+			}
+		});
+		return methods.toArray(new Method[methods.size()]);
+	}
+
 	/**
 	 * Invoke the given callback on all fields in the target class, going up the
 	 * class hierarchy to get all declared fields.

org.springframework.core/src/test/java/org/springframework/util/ReflectionUtilsTests.java

@@ -16,6 +16,9 @@
 
 package org.springframework.util;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.*;
+
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -24,7 +27,6 @@ import java.rmi.RemoteException;
 import java.util.LinkedList;
 import java.util.List;
 
-import static org.junit.Assert.*;
 import org.junit.Test;
 
 import org.springframework.beans.TestBean;
@@ -238,6 +240,88 @@ public class ReflectionUtilsTests {
 		assertNotNull(ReflectionUtils.findMethod(B.class, "getClass"));
 	}
 
+	@Test
+	public void isCglibRenamedMethod() throws SecurityException, NoSuchMethodException {
+		@SuppressWarnings("unused")
+		class C {
+			public void CGLIB$m1$123() { }
+			public void CGLIB$m1$0() { }
+			public void CGLIB$$0() { }
+			public void CGLIB$m1$() { }
+			public void CGLIB$m1() { }
+			public void m1() { }
+			public void m1$() { }
+			public void m1$1() { }
+		}
+		assertTrue(ReflectionUtils.isCglibRenamedMethod(C.class.getMethod("CGLIB$m1$123")));
+		assertTrue(ReflectionUtils.isCglibRenamedMethod(C.class.getMethod("CGLIB$m1$0")));
+		assertFalse(ReflectionUtils.isCglibRenamedMethod(C.class.getMethod("CGLIB$$0")));
+		assertFalse(ReflectionUtils.isCglibRenamedMethod(C.class.getMethod("CGLIB$m1$")));
+		assertFalse(ReflectionUtils.isCglibRenamedMethod(C.class.getMethod("CGLIB$m1")));
+		assertFalse(ReflectionUtils.isCglibRenamedMethod(C.class.getMethod("m1")));
+		assertFalse(ReflectionUtils.isCglibRenamedMethod(C.class.getMethod("m1$")));
+		assertFalse(ReflectionUtils.isCglibRenamedMethod(C.class.getMethod("m1$1")));
+	}
+
+	@Test
+	public void getAllDeclaredMethods() throws Exception {
+		class Foo {
+			@Override
+			public String toString() {
+				return super.toString();
+			}
+		}
+		int toStringMethodCount = 0;
+		for (Method method : ReflectionUtils.getAllDeclaredMethods(Foo.class)) {
+			if (method.getName().equals("toString")) {
+				toStringMethodCount++;
+			}
+		}
+		assertThat(toStringMethodCount, is(2));
+	}
+
+	@Test
+	public void getUniqueDeclaredMethods() throws Exception {
+		class Foo {
+			@Override
+			public String toString() {
+				return super.toString();
+			}
+		}
+		int toStringMethodCount = 0;
+		for (Method method : ReflectionUtils.getUniqueDeclaredMethods(Foo.class)) {
+			if (method.getName().equals("toString")) {
+				toStringMethodCount++;
+			}
+		}
+		assertThat(toStringMethodCount, is(1));
+	}
+
+	@Test
+	public void getUniqueDeclaredMethods_withCovariantReturnType() throws Exception {
+		class Parent {
+			public Number m1() {
+				return new Integer(42);
+			}
+		}
+		class Leaf extends Parent {
+			@Override
+			public Integer m1() {
+				return new Integer(42);
+			}
+		}
+		int m1MethodCount = 0;
+		Method[] methods = ReflectionUtils.getUniqueDeclaredMethods(Leaf.class);
+		for (Method method : methods) {
+			if (method.getName().equals("m1")) {
+				m1MethodCount++;
+			}
+		}
+		assertThat(m1MethodCount, is(1));
+		assertTrue(ObjectUtils.containsElement(methods, Leaf.class.getMethod("m1")));
+		assertFalse(ObjectUtils.containsElement(methods, Parent.class.getMethod("m1")));
+	}
+
 
 	private static class ListSavingMethodCallback implements ReflectionUtils.MethodCallback {