Detect a FactoryBean type match even if predictBeanType returned a non-FactoryBean type

This change should be the final piece in the puzzle to let SmartInstantiationAwareBeanPostProcessor's predictBeanType predict a FactoryBean-produced type, effectively as a semantic alternative to its postProcessBeforeInstantiation-related role.

Issue: SPR-10517

Author: jhoeller

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java

@@ -521,26 +521,32 @@ else if (containsSingleton(beanName) && !containsBeanDefinition(beanName)) {
 				}
 			}
 
-			Class<?> beanClass = predictBeanType(beanName, mbd, typesToMatch);
-			if (beanClass == null) {
+			Class<?> beanType = predictBeanType(beanName, mbd, typesToMatch);
+			if (beanType == null) {
 				return false;
 			}
 
 			// Check bean class whether we're dealing with a FactoryBean.
-			if (FactoryBean.class.isAssignableFrom(beanClass)) {
+			if (FactoryBean.class.isAssignableFrom(beanType)) {
 				if (!BeanFactoryUtils.isFactoryDereference(name)) {
 					// If it's a FactoryBean, we want to look at what it creates, not the factory class.
-					Class<?> type = getTypeForFactoryBean(beanName, mbd);
-					return (type != null && typeToMatch.isAssignableFrom(type));
-				}
-				else {
-					return typeToMatch.isAssignableFrom(beanClass);
+					beanType = getTypeForFactoryBean(beanName, mbd);
+					if (beanType == null) {
+						return false;
+					}
 				}
 			}
-			else {
-				return !BeanFactoryUtils.isFactoryDereference(name) &&
-						typeToMatch.isAssignableFrom(beanClass);
+			else if (BeanFactoryUtils.isFactoryDereference(name)) {
+				// Special case: A SmartInstantiationAwareBeanPostProcessor returned a non-FactoryBean
+				// type but we nevertheless are being asked to dereference a FactoryBean...
+				// Let's check the original bean class and proceed with it if it is a FactoryBean.
+				beanType = predictBeanType(beanName, mbd, FactoryBean.class);
+				if (beanType == null || !FactoryBean.class.isAssignableFrom(beanType)) {
+					return false;
+				}
 			}
+
+			return typeToMatch.isAssignableFrom(beanType);
 		}
 	}
 
@@ -1377,8 +1383,8 @@ protected Class<?> predictBeanType(String beanName, RootBeanDefinition mbd, Clas
 	 * @param mbd the corresponding bean definition
 	 */
 	protected boolean isFactoryBean(String beanName, RootBeanDefinition mbd) {
-		Class<?> beanClass = predictBeanType(beanName, mbd, FactoryBean.class);
-		return (beanClass != null && FactoryBean.class.isAssignableFrom(beanClass));
+		Class<?> beanType = predictBeanType(beanName, mbd, FactoryBean.class);
+		return (beanType != null && FactoryBean.class.isAssignableFrom(beanType));
 	}
 
 	/**

spring-beans/src/test/java/org/springframework/beans/factory/support/Spr8954Tests.java

@@ -16,59 +16,59 @@
 
 package org.springframework.beans.factory.support;
 
-import static org.hamcrest.CoreMatchers.*;
-import static org.junit.Assert.*;
-
+import java.util.Arrays;
 import java.util.Map;
 
+import org.junit.Before;
 import org.junit.Test;
 
-import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessor;
 import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessorAdapter;
-import org.springframework.beans.factory.support.DefaultListableBeanFactory;
-import org.springframework.beans.factory.support.RootBeanDefinition;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
 
 /**
  * Unit tests for SPR-8954, in which a custom {@link InstantiationAwareBeanPostProcessor}
  * forces the predicted type of a FactoryBean, effectively preventing retrieval of the
  * bean from calls to #getBeansOfType(FactoryBean.class). The implementation of
- * {@link AbstractBeanFactory#isFactoryBean(String, RootBeanDefinition)} now ensures
- * that not only the predicted bean type is considered, but also the original bean
- * definition's beanClass.
+ * {@link AbstractBeanFactory#isFactoryBean(String, RootBeanDefinition)} now ensures that
+ * not only the predicted bean type is considered, but also the original bean definition's
+ * beanClass.
  *
  * @author Chris Beams
  * @author Oliver Gierke
  */
 public class Spr8954Tests {
 
-	@Test
-	public void repro() {
-		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
+	private DefaultListableBeanFactory bf;
+
+	@Before
+	public void setUp() {
+		bf = new DefaultListableBeanFactory();
 		bf.registerBeanDefinition("foo", new RootBeanDefinition(FooFactoryBean.class));
 		bf.addBeanPostProcessor(new PredictingBPP());
+	}
 
+	@Test
+	public void repro() {
 		assertThat(bf.getBean("foo"), instanceOf(Foo.class));
 		assertThat(bf.getBean("&foo"), instanceOf(FooFactoryBean.class));
-
 		assertThat(bf.isTypeMatch("&foo", FactoryBean.class), is(true));
 
 		@SuppressWarnings("rawtypes")
 		Map<String, FactoryBean> fbBeans = bf.getBeansOfType(FactoryBean.class);
-		assertThat(1, equalTo(fbBeans.size()));
-		assertThat("&foo", equalTo(fbBeans.keySet().iterator().next()));
+		assertThat(fbBeans.size(), is(1));
+		assertThat(fbBeans.keySet(), hasItem("&foo"));
 
 		Map<String, AnInterface> aiBeans = bf.getBeansOfType(AnInterface.class);
-		assertThat(1, equalTo(aiBeans.size()));
-		assertThat("&foo", equalTo(aiBeans.keySet().iterator().next()));
+		assertThat(aiBeans.size(), is(1));
+		assertThat(aiBeans.keySet(), hasItem("&foo"));
 	}
 
 	@Test
 	public void findsBeansByTypeIfNotInstantiated() {
-		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
-		bf.registerBeanDefinition("foo", new RootBeanDefinition(FooFactoryBean.class));
-		bf.addBeanPostProcessor(new PredictingBPP());
-
 		assertThat(bf.isTypeMatch("&foo", FactoryBean.class), is(true));
 
 		@SuppressWarnings("rawtypes")
@@ -77,8 +77,21 @@ public void findsBeansByTypeIfNotInstantiated() {
 		assertThat("&foo", equalTo(fbBeans.keySet().iterator().next()));
 
 		Map<String, AnInterface> aiBeans = bf.getBeansOfType(AnInterface.class);
-		assertThat(1, equalTo(aiBeans.size()));
-		assertThat("&foo", equalTo(aiBeans.keySet().iterator().next()));
+		assertThat(aiBeans.size(), is(1));
+		assertThat(aiBeans.keySet(), hasItem("&foo"));
+	}
+
+	/**
+	 * SPR-10517
+	 */
+	@Test
+	public void findsFactoryBeanNameByTypeWithoutInstantiation() {
+		String[] names = bf.getBeanNamesForType(AnInterface.class, false, false);
+		assertThat(Arrays.asList(names), hasItem("&foo"));
+
+		Map<String, AnInterface> beans = bf.getBeansOfType(AnInterface.class, false, false);
+		assertThat(beans.size(), is(1));
+		assertThat(beans.keySet(), hasItem("&foo"));
 	}
 
 
@@ -116,8 +129,7 @@ static class PredictingBPP extends InstantiationAwareBeanPostProcessorAdapter {
 
 		@Override
 		public Class<?> predictBeanType(Class<?> beanClass, String beanName) {
-			return FactoryBean.class.isAssignableFrom(beanClass) ?
-					PredictedType.class : null;
+			return FactoryBean.class.isAssignableFrom(beanClass) ? PredictedType.class : null;
 		}
 	}