Refactor BeanInfoFactory

This commit refactors the BeanInfoFactory so that:

 - supports() and getBeanInfo() are folded into one, so that getBeanInfo()
    returns null if a given class is not supported.
 - CachedIntrospectionResults now uses SpringFactoriesLoader

Author: poutsma

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

@@ -23,17 +23,18 @@
  * Strategy for creating {@link BeanInfo} instances.
  *
  * <p>BeanInfoFactories are are instantiated by the {@link CachedIntrospectionResults},
- * which looks for {@code META-INF/spring.beanInfoFactories} files on the class path.
- * These files contain one or more {@code BeanInfoFactory} class names, each of a single
- * line. When a {@link BeanInfo} is to be created, the {@code CachedIntrospectionResults}
- * will iterate through the discovered factories, asking each one if it {@linkplain
- * #supports(Class) supports} the given bean class. If it does, {@link
- * #getBeanInfo(Class)} will be called; if not, the next factory will be queried. If none
- * of the factories support the class, an standard {@link BeanInfo} is created as a
- * default.
+ * by using the {@link org.springframework.core.io.support.SpringFactoriesLoader} utility
+ * class.
  *
- * <p>Note that the {@link CachedIntrospectionResults} sorts the {@code BeanInfoFactory}
- * instances by {@link org.springframework.core.annotation.Order Order}, so that ones with
+ * When a {@link BeanInfo} is to be created, the {@code CachedIntrospectionResults}
+ * will iterate through the discovered factories, calling {@link
+ * #getBeanInfo(Class)} on each one. If {@code null} is returned, the next factory will
+ * be queried. If none of the factories support the class, an standard {@link BeanInfo}
+ * is created as a default.
+ *
+ * <p>Note that the {@link org.springframework.core.io.support.SpringFactoriesLoader}
+ * sorts the {@code BeanInfoFactory} instances by
+ * {@link org.springframework.core.annotation.Order @Order}, so that ones with
  * a higher precedence come first.
  *
  * @author Arjen Poutsma
@@ -42,18 +43,10 @@
 public interface BeanInfoFactory {
 
 	/**
-	 * Indicates whether a bean with the given class is supported by this factory.
-	 *
-	 * @param beanClass the bean class
-	 * @return {@code true} if supported; {@code false} otherwise
-	 */
-	boolean supports(Class<?> beanClass);
-
-	/**
-	 * Returns the bean info for the given class.
+	 * Returns the bean info for the given class, if supported.
 	 *
 	 * @param beanClass the bean class
-	 * @return the bean info
+	 * @return the bean info, or {@code null} if not the given class is not supported
 	 * @throws IntrospectionException in case of exceptions
 	 */
 	BeanInfo getBeanInfo(Class<?> beanClass) throws IntrospectionException;

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

@@ -20,25 +20,21 @@
 import java.beans.IntrospectionException;
 import java.beans.Introspector;
 import java.beans.PropertyDescriptor;
-import java.io.IOException;
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Properties;
 import java.util.Set;
 import java.util.WeakHashMap;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.springframework.core.annotation.AnnotationAwareOrderComparator;
-import org.springframework.core.io.support.PropertiesLoaderUtils;
+import org.springframework.core.io.support.SpringFactoriesLoader;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
 
@@ -64,12 +60,6 @@
  */
 public class CachedIntrospectionResults {
 
-	/**
-	 * The location to look for the bean info mapping files. Can be present in multiple JAR files.
-	 */
-	public static final String BEAN_INFO_FACTORIES_LOCATION = "META-INF/spring.beanInfoFactories";
-
-
 	private static final Log logger = LogFactory.getLog(CachedIntrospectionResults.class);
 
 	/**
@@ -242,8 +232,8 @@ private CachedIntrospectionResults(Class beanClass, boolean cacheFullMetadata) t
 			BeanInfo beanInfo = null;
 			List<BeanInfoFactory> beanInfoFactories = getBeanInfoFactories(beanClass.getClassLoader());
 			for (BeanInfoFactory beanInfoFactory : beanInfoFactories) {
-				if (beanInfoFactory.supports(beanClass)) {
-					beanInfo = beanInfoFactory.getBeanInfo(beanClass);
+				beanInfo = beanInfoFactory.getBeanInfo(beanClass);
+				if (beanInfo != null) {
 					break;
 				}
 			}
@@ -339,57 +329,12 @@ private static List<BeanInfoFactory> getBeanInfoFactories(ClassLoader classLoade
 		if (beanInfoFactories == null) {
 			synchronized (beanInfoFactoriesMutex) {
 				if (beanInfoFactories == null) {
-					try {
-						Properties properties =
-								PropertiesLoaderUtils.loadAllProperties(
-										BEAN_INFO_FACTORIES_LOCATION, classLoader);
-
-						if (logger.isDebugEnabled()) {
-							logger.debug("Loaded BeanInfoFactories: " + properties.keySet());
-						}
-
-						List<BeanInfoFactory> factories = new ArrayList<BeanInfoFactory>(properties.size());
-
-						for (Object key : properties.keySet()) {
-							if (key instanceof String) {
-								String className = (String) key;
-								BeanInfoFactory factory = instantiateBeanInfoFactory(className, classLoader);
-								factories.add(factory);
-							}
-						}
-
-						Collections.sort(factories, new AnnotationAwareOrderComparator());
-
-						beanInfoFactories = Collections.synchronizedList(factories);
-					}
-					catch (IOException ex) {
-						throw new IllegalStateException(
-								"Unable to load BeanInfoFactories from location [" + BEAN_INFO_FACTORIES_LOCATION + "]", ex);
-					}
+					beanInfoFactories = Collections.synchronizedList(SpringFactoriesLoader
+							.loadFactories(BeanInfoFactory.class, classLoader));
 				}
 			}
 		}
 		return beanInfoFactories;
 	}
 
-	private static BeanInfoFactory instantiateBeanInfoFactory(String className,
-	                                                          ClassLoader classLoader) {
-		try {
-			Class<?> factoryClass = ClassUtils.forName(className, classLoader);
-			if (!BeanInfoFactory.class.isAssignableFrom(factoryClass)) {
-				throw new FatalBeanException(
-						"Class [" + className + "] does not implement the [" +
-								BeanInfoFactory.class.getName() + "] interface");
-			}
-			return (BeanInfoFactory) BeanUtils.instantiate(factoryClass);
-		}
-		catch (ClassNotFoundException ex) {
-			throw new FatalBeanException(
-					"BeanInfoFactory class [" + className + "] not found", ex);
-		}
-		catch (LinkageError err) {
-			throw new FatalBeanException("Invalid BeanInfoFactory class [" + className +
-					"]: problem with handler class file or dependent class", err);
-		}
-	}
 }

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

@@ -19,7 +19,6 @@
 import java.beans.BeanInfo;
 import java.beans.IntrospectionException;
 import java.beans.Introspector;
-
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 
@@ -37,13 +36,21 @@
  * @since 3.2
  * @see BeanInfoFactory
  */
-class ExtendedBeanInfoFactory implements Ordered, BeanInfoFactory {
+public class ExtendedBeanInfoFactory implements Ordered, BeanInfoFactory {
+
+	/**
+	 * Return a new {@link ExtendedBeanInfo} for the given bean class.
+	 */
+	public BeanInfo getBeanInfo(Class<?> beanClass) throws IntrospectionException {
+		return supports(beanClass) ?
+				new ExtendedBeanInfo(Introspector.getBeanInfo(beanClass)) : null;
+	}
 
 	/**
 	 * Return whether the given bean class declares or inherits any non-void returning
 	 * JavaBeans or <em>indexed property</em> setter methods.
 	 */
-	public boolean supports(Class<?> beanClass) {
+	private boolean supports(Class<?> beanClass) {
 		for (Method method : beanClass.getMethods()) {
 			String methodName = method.getName();
 			Class<?>[] parameterTypes = method.getParameterTypes();
@@ -59,13 +66,6 @@ public boolean supports(Class<?> beanClass) {
 		return false;
 	}
 
-	/**
-	 * Return a new {@link ExtendedBeanInfo} for the given bean class.
-	 */
-	public BeanInfo getBeanInfo(Class<?> beanClass) throws IntrospectionException {
-		return new ExtendedBeanInfo(Introspector.getBeanInfo(beanClass));
-	}
-
 	public int getOrder() {
 		return Ordered.LOWEST_PRECEDENCE;
 	}

spring-beans/src/main/resources/META-INF/spring.beanInfoFactories

@@ -0,0 +1 @@
+org.springframework.beans.BeanInfoFactory=org.springframework.beans.ExtendedBeanInfoFactory

spring-beans/src/main/resources/META-INF/spring.factories

@@ -18,11 +18,11 @@
 
 import java.beans.IntrospectionException;
 
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.junit.Assert.assertThat;
 import org.junit.Test;
 
-import static org.hamcrest.CoreMatchers.*;
-import static org.junit.Assert.*;
-
 /**
  * Unit tests for {@link ExtendedBeanInfoTests}.
  *
@@ -38,7 +38,7 @@ public void shouldNotSupportClassHavingOnlyVoidReturningSetter() throws Introspe
 		class C {
 			public void setFoo(String s) { }
 		}
-		assertThat(factory.supports(C.class), is(false));
+		assertThat(factory.getBeanInfo(C.class), nullValue());
 	}
 
 	@Test
@@ -47,7 +47,7 @@ public void shouldSupportClassHavingNonVoidReturningSetter() throws Introspectio
 		class C {
 			public C setFoo(String s) { return this; }
 		}
-		assertThat(factory.supports(C.class), is(true));
+		assertThat(factory.getBeanInfo(C.class), notNullValue());
 	}
 
 	@Test
@@ -56,7 +56,7 @@ public void shouldSupportClassHavingNonVoidReturningIndexedSetter() throws Intro
 		class C {
 			public C setFoo(int i, String s) { return this; }
 		}
-		assertThat(factory.supports(C.class), is(true));
+		assertThat(factory.getBeanInfo(C.class), notNullValue());
 	}
 
 	@Test
@@ -65,7 +65,7 @@ public void shouldNotSupportClassHavingNonPublicNonVoidReturningIndexedSetter()
 		class C {
 			void setBar(String s) { }
 		}
-		assertThat(factory.supports(C.class), is(false));
+		assertThat(factory.getBeanInfo(C.class), nullValue());
 	}
 
 	@Test
@@ -74,7 +74,7 @@ public void shouldNotSupportClassHavingNonVoidReturningParameterlessSetter() thr
 		class C {
 			C setBar() { return this; }
 		}
-		assertThat(factory.supports(C.class), is(false));
+		assertThat(factory.getBeanInfo(C.class), nullValue());
 	}
 
 	@Test
@@ -83,7 +83,7 @@ public void shouldNotSupportClassHavingNonVoidReturningMethodNamedSet() throws I
 		class C {
 			C set(String s) { return this; }
 		}
-		assertThat(factory.supports(C.class), is(false));
+		assertThat(factory.getBeanInfo(C.class), nullValue());
 	}
 
 }