Check BeanInfoFactory for interface introspection as well

Issue: SPR-16322

Author: jhoeller

spring-beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -244,6 +244,27 @@ private static boolean isUnderneathClassLoader(@Nullable ClassLoader candidate,
 		return false;
 	}
 
+	/**
+	 * Retrieve a {@link BeanInfo} descriptor for the given target class.
+	 * @param beanClass the target class to introspect
+	 * @param ignoreBeaninfoClasses whether to apply {@link Introspector#IGNORE_ALL_BEANINFO} mode
+	 * @return the resulting {@code BeanInfo} descriptor (never {@code null})
+	 * @throws IntrospectionException from the underlying {@link Introspector}
+	 */
+	private static BeanInfo getBeanInfo(Class<?> beanClass, boolean ignoreBeaninfoClasses)
+			throws IntrospectionException {
+
+		for (BeanInfoFactory beanInfoFactory : beanInfoFactories) {
+			BeanInfo beanInfo = beanInfoFactory.getBeanInfo(beanClass);
+			if (beanInfo != null) {
+				return beanInfo;
+			}
+		}
+		return (ignoreBeaninfoClasses ?
+				Introspector.getBeanInfo(beanClass, Introspector.IGNORE_ALL_BEANINFO) :
+				Introspector.getBeanInfo(beanClass));
+	}
+
 
 	/** The BeanInfo object for the introspected bean class */
 	private final BeanInfo beanInfo;
@@ -265,21 +286,7 @@ private CachedIntrospectionResults(Class<?> beanClass) throws BeansException {
 			if (logger.isTraceEnabled()) {
 				logger.trace("Getting BeanInfo for class [" + beanClass.getName() + "]");
 			}
-
-			BeanInfo beanInfo = null;
-			for (BeanInfoFactory beanInfoFactory : beanInfoFactories) {
-				beanInfo = beanInfoFactory.getBeanInfo(beanClass);
-				if (beanInfo != null) {
-					break;
-				}
-			}
-			if (beanInfo == null) {
-				// If none of the factories supported the class, fall back to the default
-				beanInfo = (shouldIntrospectorIgnoreBeaninfoClasses ?
-						Introspector.getBeanInfo(beanClass, Introspector.IGNORE_ALL_BEANINFO) :
-						Introspector.getBeanInfo(beanClass));
-			}
-			this.beanInfo = beanInfo;
+			this.beanInfo = getBeanInfo(beanClass, shouldIntrospectorIgnoreBeaninfoClasses);
 
 			if (logger.isTraceEnabled()) {
 				logger.trace("Caching PropertyDescriptors for class [" + beanClass.getName() + "]");
@@ -307,15 +314,17 @@ private CachedIntrospectionResults(Class<?> beanClass) throws BeansException {
 			// Explicitly check implemented interfaces for setter/getter methods as well,
 			// in particular for Java 8 default methods...
 			Class<?> clazz = beanClass;
-			while (clazz != null) {
+			while (clazz != null && clazz != Object.class) {
 				Class<?>[] ifcs = clazz.getInterfaces();
 				for (Class<?> ifc : ifcs) {
-					BeanInfo ifcInfo = Introspector.getBeanInfo(ifc, Introspector.IGNORE_ALL_BEANINFO);
-					PropertyDescriptor[] ifcPds = ifcInfo.getPropertyDescriptors();
-					for (PropertyDescriptor pd : ifcPds) {
-						if (!this.propertyDescriptorCache.containsKey(pd.getName())) {
-							pd = buildGenericTypeAwarePropertyDescriptor(beanClass, pd);
-							this.propertyDescriptorCache.put(pd.getName(), pd);
+					if (!ClassUtils.isJavaLanguageInterface(ifc)) {
+						BeanInfo ifcInfo = getBeanInfo(ifc, true);
+						PropertyDescriptor[] ifcPds = ifcInfo.getPropertyDescriptors();
+						for (PropertyDescriptor pd : ifcPds) {
+							if (!this.propertyDescriptorCache.containsKey(pd.getName())) {
+								pd = buildGenericTypeAwarePropertyDescriptor(beanClass, pd);
+								this.propertyDescriptorCache.put(pd.getName(), pd);
+							}
 						}
 					}
 				}
@@ -329,6 +338,7 @@ private CachedIntrospectionResults(Class<?> beanClass) throws BeansException {
 		}
 	}
 
+
 	BeanInfo getBeanInfo() {
 		return this.beanInfo;
 	}

spring-core/src/main/java/org/springframework/core/Conventions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -16,16 +16,10 @@
 
 package org.springframework.core;
 
-import java.io.Externalizable;
-import java.io.Serializable;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
-import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
 import java.util.Iterator;
-import java.util.Set;
 
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
@@ -47,36 +41,21 @@ public abstract class Conventions {
 	 */
 	private static final String PLURAL_SUFFIX = "List";
 
-	/**
-	 * Set of interfaces that are supposed to be ignored
-	 * when searching for the 'primary' interface of a proxy.
-	 */
-	private static final Set<Class<?>> IGNORED_INTERFACES;
-
-	static {
-		IGNORED_INTERFACES = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
-				Serializable.class, Externalizable.class, Cloneable.class, Comparable.class)));
-	}
-
-	private static final ReactiveAdapterRegistry reactiveAdapterRegistry =
-			ReactiveAdapterRegistry.getSharedInstance();
+	private static final ReactiveAdapterRegistry reactiveAdapterRegistry = ReactiveAdapterRegistry.getSharedInstance();
 
 
 	/**
 	 * Determine the conventional variable name for the supplied {@code Object}
 	 * based on its concrete type. The convention used is to return the
 	 * un-capitalized short name of the {@code Class}, according to JavaBeans
 	 * property naming rules.
-	 *
 	 * <p>For example:<br>
 	 * {@code com.myapp.Product} becomes {@code "product"}<br>
 	 * {@code com.myapp.MyProduct} becomes {@code "myProduct"}<br>
 	 * {@code com.myapp.UKProduct} becomes {@code "UKProduct"}<br>
-	 *
 	 * <p>For arrays the pluralized version of the array component type is used.
 	 * For {@code Collection}s an attempt is made to 'peek ahead' to determine
 	 * the component type and return its pluralized version.
-	 *
 	 * @param value the value to generate a variable name for
 	 * @return the generated variable name
 	 */
@@ -110,12 +89,10 @@ else if (value instanceof Collection) {
 	/**
 	 * Determine the conventional variable name for the given parameter taking
 	 * the generic collection type, if any, into account.
-	 *
 	 * <p>As of 5.0 this method supports reactive types:<br>
 	 * {@code Mono<com.myapp.Product>} becomes {@code "productMono"}<br>
 	 * {@code Flux<com.myapp.MyProduct>} becomes {@code "myProductFlux"}<br>
 	 * {@code Observable<com.myapp.MyProduct>} becomes {@code "myProductObservable"}<br>
-	 *
 	 * @param parameter the method or constructor parameter
 	 * @return the generated variable name
 	 */
@@ -295,7 +272,7 @@ private static Class<?> getClassForValue(Object value) {
 		if (Proxy.isProxyClass(valueClass)) {
 			Class<?>[] ifcs = valueClass.getInterfaces();
 			for (Class<?> ifc : ifcs) {
-				if (!IGNORED_INTERFACES.contains(ifc)) {
+				if (!ClassUtils.isJavaLanguageInterface(ifc)) {
 					return ifc;
 				}
 			}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -17,6 +17,8 @@
 package org.springframework.util;
 
 import java.beans.Introspector;
+import java.io.Externalizable;
+import java.io.Serializable;
 import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
@@ -74,6 +76,13 @@ public abstract class ClassUtils {
 	public static final String CLASS_FILE_SUFFIX = ".class";
 
 
+	/**
+	 * Common Java language interfaces which are supposed to be ignored
+	 * when searching for 'primary' user-level interfaces.
+	 */
+	private static final Set<Class<?>> javaLanguageInterfaces = new HashSet<>(
+			Arrays.asList(Serializable.class, Externalizable.class, Cloneable.class, Comparable.class));
+
 	/**
 	 * Map with primitive wrapper type as key and corresponding primitive
 	 * type as value, for example: Integer.class -> int.class.
@@ -1218,6 +1227,19 @@ public static boolean isVisible(Class<?> clazz, @Nullable ClassLoader classLoade
 		}
 	}
 
+	/**
+	 * Determine whether the given interface is a common Java language interface:
+	 * {@link Serializable}, {@link Externalizable}, {@link Cloneable}, {@link Comparable}
+	 * - all of which can be ignored when looking for 'primary' user-level interfaces.
+	 * Common characteristics: no service-level operations, no bean property methods,
+	 * no default methods.
+	 * @param ifc the interface to check
+	 * @since 5.0.3
+	 */
+	public static boolean isJavaLanguageInterface(Class<?> ifc) {
+		return javaLanguageInterfaces.contains(ifc);
+	}
+
 	/**
 	 * Check whether the given object is a CGLIB proxy.
 	 * @param object the object to check