Consistent support for Java 8 default methods (in interfaces implemented by user classes)

Covers ReflectionUtils.doWithMethods as well as affected annotation post-processors.
Includes an extension of MethodMetadata for the detection of @Bean default methods.

Issue: SPR-12822
Issue: SPR-10919

spring-core/src/main/java/org/springframework/core/type/MethodMetadata.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -41,6 +41,14 @@ public interface MethodMetadata extends AnnotatedTypeMetadata {
 	 */
 	public String getDeclaringClassName();
 
+	/**
+	 * Return whether the underlying method is effectively abstract:
+	 * i.e. marked as abstract on a class or declared as a regular,
+	 * non-default method in an interface.
+	 * @since 4.2
+	 */
+	boolean isAbstract();
+
 	/**
 	 * Return whether the underlying method is declared as 'static'.
 	 */

spring-core/src/main/java/org/springframework/core/type/StandardMethodMetadata.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -84,6 +84,11 @@ public class StandardMethodMetadata implements MethodMetadata {
 		return this.introspectedMethod.getDeclaringClass().getName();
 	}
 
+	@Override
+	public boolean isAbstract() {
+		return Modifier.isAbstract(this.introspectedMethod.getModifiers());
+	}
+
 	@Override
 	public boolean isStatic() {
 		return Modifier.isStatic(this.introspectedMethod.getModifiers());

spring-core/src/main/java/org/springframework/core/type/classreading/MethodMetadataReadingVisitor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -82,6 +82,11 @@ public class MethodMetadataReadingVisitor extends MethodVisitor implements Metho
 		return this.name;
 	}
 
+	@Override
+	public boolean isAbstract() {
+		return ((this.access & Opcodes.ACC_ABSTRACT) != 0);
+	}
+
 	@Override
 	public boolean isStatic() {
 		return ((this.access & Opcodes.ACC_STATIC) != 0);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -25,6 +25,7 @@ import java.lang.reflect.UndeclaredThrowableException;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
@@ -51,11 +52,18 @@ public abstract class ReflectionUtils {
 	private static final String CGLIB_RENAMED_METHOD_PREFIX = "CGLIB$";
 
 	/**
-	 * Cache for {@link Class#getDeclaredMethods()}, allowing for fast resolution.
+	 * Cache for {@link Class#getDeclaredMethods()} plus equivalent default methods
+	 * from Java 8 based interfaces, allowing for fast iteration.
 	 */
 	private static final Map<Class<?>, Method[]> declaredMethodsCache =
 			new ConcurrentReferenceHashMap<Class<?>, Method[]>(256);
 
+	/**
+	 * Cache for {@link Class#getDeclaredFields()}, allowing for fast iteration.
+	 */
+	private static final Map<Class<?>, Field[]> declaredFieldsCache =
+			new ConcurrentReferenceHashMap<Class<?>, Field[]>(256);
+
 
 	/**
 	 * Attempt to find a {@link Field field} on the supplied {@link Class} with the
@@ -82,7 +90,7 @@ public abstract class ReflectionUtils {
 		Assert.isTrue(name != null || type != null, "Either name or type of the field must be specified");
 		Class<?> searchType = clazz;
 		while (!Object.class.equals(searchType) && searchType != null) {
-			Field[] fields = searchType.getDeclaredFields();
+			Field[] fields = getDeclaredFields(searchType);
 			for (Field field : fields) {
 				if ((name == null || name.equals(field.getName())) && (type == null || type.equals(field.getType()))) {
 					return field;
@@ -440,8 +448,8 @@ public abstract class ReflectionUtils {
 	 * @see java.lang.reflect.Method#setAccessible
 	 */
 	public static void makeAccessible(Method method) {
-		if ((!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers())) &&
-				!method.isAccessible()) {
+		if ((!Modifier.isPublic(method.getModifiers()) ||
+				!Modifier.isPublic(method.getDeclaringClass().getModifiers())) && !method.isAccessible()) {
 			method.setAccessible(true);
 		}
 	}
@@ -455,22 +463,43 @@ public abstract class ReflectionUtils {
 	 * @see java.lang.reflect.Constructor#setAccessible
 	 */
 	public static void makeAccessible(Constructor<?> ctor) {
-		if ((!Modifier.isPublic(ctor.getModifiers()) || !Modifier.isPublic(ctor.getDeclaringClass().getModifiers())) &&
-				!ctor.isAccessible()) {
+		if ((!Modifier.isPublic(ctor.getModifiers()) ||
+				!Modifier.isPublic(ctor.getDeclaringClass().getModifiers())) && !ctor.isAccessible()) {
 			ctor.setAccessible(true);
 		}
 	}
 
+	/**
+	 * Perform the given callback operation on all matching methods of the given
+	 * class, as locally declared or equivalent thereof (such as default methods
+	 * on Java 8 based interfaces that the given class implements).
+	 * @param clazz the class to introspect
+	 * @param mc the callback to invoke for each method
+	 * @since 4.2
+	 * @see #doWithMethods
+	 */
+	public static void doWithLocalMethods(Class<?> clazz, MethodCallback mc) {
+		Method[] methods = getDeclaredMethods(clazz);
+		for (Method method : methods) {
+			try {
+				mc.doWith(method);
+			}
+			catch (IllegalAccessException ex) {
+				throw new IllegalStateException("Not allowed to access method '" + method.getName() + "': " + ex);
+			}
+		}
+	}
+
 	/**
 	 * Perform the given callback operation on all matching methods of the given
 	 * class and superclasses.
 	 * <p>The same named method occurring on subclass and superclass will appear
 	 * twice, unless excluded by a {@link MethodFilter}.
-	 * @param clazz class to start looking at
+	 * @param clazz the class to introspect
 	 * @param mc the callback to invoke for each method
 	 * @see #doWithMethods(Class, MethodCallback, MethodFilter)
 	 */
-	public static void doWithMethods(Class<?> clazz, MethodCallback mc) throws IllegalArgumentException {
+	public static void doWithMethods(Class<?> clazz, MethodCallback mc) {
 		doWithMethods(clazz, mc, null);
 	}
 
@@ -479,13 +508,11 @@ public abstract class ReflectionUtils {
 	 * class and superclasses (or given interface and super-interfaces).
 	 * <p>The same named method occurring on subclass and superclass will appear
 	 * twice, unless excluded by the specified {@link MethodFilter}.
-	 * @param clazz class to start looking at
+	 * @param clazz the class to introspect
 	 * @param mc the callback to invoke for each method
 	 * @param mf the filter that determines the methods to apply the callback to
 	 */
-	public static void doWithMethods(Class<?> clazz, MethodCallback mc, MethodFilter mf)
-			throws IllegalArgumentException {
-
+	public static void doWithMethods(Class<?> clazz, MethodCallback mc, MethodFilter mf) {
 		// Keep backing up the inheritance hierarchy.
 		Method[] methods = getDeclaredMethods(clazz);
 		for (Method method : methods) {
@@ -496,7 +523,7 @@ public abstract class ReflectionUtils {
 				mc.doWith(method);
 			}
 			catch (IllegalAccessException ex) {
-				throw new IllegalStateException("Shouldn't be illegal to access method '" + method.getName() + "': " + ex);
+				throw new IllegalStateException("Not allowed to access method '" + method.getName() + "': " + ex);
 			}
 		}
 		if (clazz.getSuperclass() != null) {
@@ -510,10 +537,11 @@ public abstract class ReflectionUtils {
 	}
 
 	/**
-	 * Get all declared methods on the leaf class and all superclasses. Leaf
-	 * class methods are included first.
+	 * Get all declared methods on the leaf class and all superclasses.
+	 * Leaf class methods are included first.
+	 * @param leafClass the class to introspect
 	 */
-	public static Method[] getAllDeclaredMethods(Class<?> leafClass) throws IllegalArgumentException {
+	public static Method[] getAllDeclaredMethods(Class<?> leafClass) {
 		final List<Method> methods = new ArrayList<Method>(32);
 		doWithMethods(leafClass, new MethodCallback() {
 			@Override
@@ -525,11 +553,12 @@ public abstract class ReflectionUtils {
 	}
 
 	/**
-	 * 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.
+	 * 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.
+	 * @param leafClass the class to introspect
 	 */
-	public static Method[] getUniqueDeclaredMethods(Class<?> leafClass) throws IllegalArgumentException {
+	public static Method[] getUniqueDeclaredMethods(Class<?> leafClass) {
 		final List<Method> methods = new ArrayList<Method>(32);
 		doWithMethods(leafClass, new MethodCallback() {
 			@Override
@@ -562,25 +591,77 @@ public abstract class ReflectionUtils {
 	}
 
 	/**
-	 * This method retrieves {@link Class#getDeclaredMethods()} from a local cache
+	 * This variant retrieves {@link Class#getDeclaredMethods()} from a local cache
 	 * in order to avoid the JVM's SecurityManager check and defensive array copying.
+	 * In addition, it also includes Java 8 default methods from locally implemented
+	 * interfaces, since those are effectively to be treated just like declared methods.
+	 * @param clazz the class to introspect
+	 * @return the cached array of methods
+	 * @see Class#getDeclaredMethods()
 	 */
 	private static Method[] getDeclaredMethods(Class<?> clazz) {
 		Method[] result = declaredMethodsCache.get(clazz);
 		if (result == null) {
-			result = clazz.getDeclaredMethods();
+			Method[] declaredMethods = clazz.getDeclaredMethods();
+			List<Method> defaultMethods = findConcreteMethodsOnInterfaces(clazz);
+			if (defaultMethods != null) {
+				result = new Method[declaredMethods.length + defaultMethods.size()];
+				System.arraycopy(declaredMethods, 0, result, 0, declaredMethods.length);
+				int index = declaredMethods.length;
+				for (Method defaultMethod : defaultMethods) {
+					result[index] = defaultMethod;
+					index++;
+				}
+			}
+			else {
+				result = declaredMethods;
+			}
 			declaredMethodsCache.put(clazz, result);
 		}
 		return result;
 	}
 
+	private static List<Method> findConcreteMethodsOnInterfaces(Class<?> clazz) {
+		List<Method> result = null;
+		for (Class<?> ifc : clazz.getInterfaces()) {
+			for (Method ifcMethod : ifc.getMethods()) {
+				if (!Modifier.isAbstract(ifcMethod.getModifiers())) {
+					if (result == null) {
+						result = new LinkedList<Method>();
+					}
+					result.add(ifcMethod);
+				}
+			}
+		}
+		return result;
+	}
+
 	/**
 	 * Invoke the given callback on all fields in the target class, going up the
 	 * class hierarchy to get all declared fields.
 	 * @param clazz the target class to analyze
 	 * @param fc the callback to invoke for each field
+	 * @since 4.2
+	 * @see #doWithFields
 	 */
-	public static void doWithFields(Class<?> clazz, FieldCallback fc) throws IllegalArgumentException {
+	public static void doWithLocalFields(Class<?> clazz, FieldCallback fc) {
+		for (Field field : getDeclaredFields(clazz)) {
+			try {
+				fc.doWith(field);
+			}
+			catch (IllegalAccessException ex) {
+				throw new IllegalStateException("Not allowed to access field '" + field.getName() + "': " + ex);
+			}
+		}
+	}
+
+	/**
+	 * Invoke the given callback on all fields in the target class, going up the
+	 * class hierarchy to get all declared fields.
+	 * @param clazz the target class to analyze
+	 * @param fc the callback to invoke for each field
+	 */
+	public static void doWithFields(Class<?> clazz, FieldCallback fc) {
 		doWithFields(clazz, fc, null);
 	}
 
@@ -591,15 +672,12 @@ public abstract class ReflectionUtils {
 	 * @param fc the callback to invoke for each field
 	 * @param ff the filter that determines the fields to apply the callback to
 	 */
-	public static void doWithFields(Class<?> clazz, FieldCallback fc, FieldFilter ff)
-			throws IllegalArgumentException {
-
+	public static void doWithFields(Class<?> clazz, FieldCallback fc, FieldFilter ff) {
 		// Keep backing up the inheritance hierarchy.
 		Class<?> targetClass = clazz;
 		do {
-			Field[] fields = targetClass.getDeclaredFields();
+			Field[] fields = getDeclaredFields(targetClass);
 			for (Field field : fields) {
-				// Skip static and final fields.
 				if (ff != null && !ff.matches(field)) {
 					continue;
 				}
@@ -607,7 +685,7 @@ public abstract class ReflectionUtils {
 					fc.doWith(field);
 				}
 				catch (IllegalAccessException ex) {
-					throw new IllegalStateException("Shouldn't be illegal to access field '" + field.getName() + "': " + ex);
+					throw new IllegalStateException("Not allowed to access field '" + field.getName() + "': " + ex);
 				}
 			}
 			targetClass = targetClass.getSuperclass();
@@ -615,13 +693,28 @@ public abstract class ReflectionUtils {
 		while (targetClass != null && targetClass != Object.class);
 	}
 
+	/**
+	 * This variant retrieves {@link Class#getDeclaredFields()} from a local cache
+	 * in order to avoid the JVM's SecurityManager check and defensive array copying.
+	 * @param clazz the class to introspect
+	 * @return the cached array of fields
+	 * @see Class#getDeclaredFields()
+	 */
+	private static Field[] getDeclaredFields(Class<?> clazz) {
+		Field[] result = declaredFieldsCache.get(clazz);
+		if (result == null) {
+			result = clazz.getDeclaredFields();
+			declaredFieldsCache.put(clazz, result);
+		}
+		return result;
+	}
+
 	/**
 	 * Given the source object and the destination, which must be the same class
 	 * or a subclass, copy all fields, including inherited fields. Designed to
 	 * work on objects with public no-arg constructors.
-	 * @throws IllegalArgumentException if the arguments are incompatible
 	 */
-	public static void shallowCopyFieldState(final Object src, final Object dest) throws IllegalArgumentException {
+	public static void shallowCopyFieldState(final Object src, final Object dest) {
 		if (src == null) {
 			throw new IllegalArgumentException("Source for field copy cannot be null");
 		}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -66,8 +66,8 @@ public class ReflectionUtilsTests {
 
 	@Test
 	public void setField() {
-		final TestObjectSubclassWithNewField testBean = new TestObjectSubclassWithNewField();
-		final Field field = ReflectionUtils.findField(TestObjectSubclassWithNewField.class, "name", String.class);
+		TestObjectSubclassWithNewField testBean = new TestObjectSubclassWithNewField();
+		Field field = ReflectionUtils.findField(TestObjectSubclassWithNewField.class, "name", String.class);
 
 		ReflectionUtils.makeAccessible(field);
 
@@ -79,13 +79,6 @@ public class ReflectionUtilsTests {
 		assertNull(testBean.getName());
 	}
 
-	@Test(expected = IllegalStateException.class)
-	public void setFieldIllegal() {
-		final TestObjectSubclassWithNewField testBean = new TestObjectSubclassWithNewField();
-		final Field field = ReflectionUtils.findField(TestObjectSubclassWithNewField.class, "name", String.class);
-		ReflectionUtils.setField(field, testBean, "FooBar");
-	}
-
 	@Test
 	public void invokeMethod() throws Exception {
 		String rob = "Rob Harrop";
@@ -94,65 +87,50 @@ public class ReflectionUtilsTests {
 		bean.setName(rob);
 
 		Method getName = TestObject.class.getMethod("getName", (Class[]) null);
-		Method setName = TestObject.class.getMethod("setName", new Class[] { String.class });
+		Method setName = TestObject.class.getMethod("setName", String.class);
 
 		Object name = ReflectionUtils.invokeMethod(getName, bean);
 		assertEquals("Incorrect name returned", rob, name);
 
 		String juergen = "Juergen Hoeller";
-		ReflectionUtils.invokeMethod(setName, bean, new Object[] { juergen });
+		ReflectionUtils.invokeMethod(setName, bean, juergen);
 		assertEquals("Incorrect name set", juergen, bean.getName());
 	}
 
 	@Test
 	public void declaresException() throws Exception {
-		Method remoteExMethod = A.class.getDeclaredMethod("foo", new Class[] { Integer.class });
+		Method remoteExMethod = A.class.getDeclaredMethod("foo", Integer.class);
 		assertTrue(ReflectionUtils.declaresException(remoteExMethod, RemoteException.class));
 		assertTrue(ReflectionUtils.declaresException(remoteExMethod, ConnectException.class));
 		assertFalse(ReflectionUtils.declaresException(remoteExMethod, NoSuchMethodException.class));
 		assertFalse(ReflectionUtils.declaresException(remoteExMethod, Exception.class));
 
-		Method illegalExMethod = B.class.getDeclaredMethod("bar", new Class[] { String.class });
+		Method illegalExMethod = B.class.getDeclaredMethod("bar", String.class);
 		assertTrue(ReflectionUtils.declaresException(illegalExMethod, IllegalArgumentException.class));
 		assertTrue(ReflectionUtils.declaresException(illegalExMethod, NumberFormatException.class));
 		assertFalse(ReflectionUtils.declaresException(illegalExMethod, IllegalStateException.class));
 		assertFalse(ReflectionUtils.declaresException(illegalExMethod, Exception.class));
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
 	public void copySrcToDestinationOfIncorrectClass() {
 		TestObject src = new TestObject();
 		String dest = new String();
-		try {
-			ReflectionUtils.shallowCopyFieldState(src, dest);
-			fail();
-		} catch (IllegalArgumentException ex) {
-			// Ok
-		}
+		ReflectionUtils.shallowCopyFieldState(src, dest);
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
 	public void rejectsNullSrc() {
 		TestObject src = null;
 		String dest = new String();
-		try {
-			ReflectionUtils.shallowCopyFieldState(src, dest);
-			fail();
-		} catch (IllegalArgumentException ex) {
-			// Ok
-		}
+		ReflectionUtils.shallowCopyFieldState(src, dest);
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
 	public void rejectsNullDest() {
 		TestObject src = new TestObject();
 		String dest = null;
-		try {
-			ReflectionUtils.shallowCopyFieldState(src, dest);
-			fail();
-		} catch (IllegalArgumentException ex) {
-			// Ok
-		}
+		ReflectionUtils.shallowCopyFieldState(src, dest);
 	}
 
 	@Test