DATACMNS-821 - Improved type prediction for FactoryBean implementations.

Previously, we registered an InstantiationAwareBeanPostProcessor to predict the type to be created by RepositoryFactoryBeanSupport by inspecting a particular property value of the registered BeanDefinition.

This has now been elevated to a more generic mechanism that can get a FactoryBean type configured with a set of properties to inspect for a configured type. That new infrastructure now replaces the explicit configuration for RepositoryFactoryBeanSupport with one that's set up via configuration.

src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupport.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2014 the original author or authors.
+ * Copyright 2012-2016 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.
@@ -29,11 +29,11 @@ import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
-import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.data.repository.core.RepositoryMetadata;
 import org.springframework.data.repository.core.support.AbstractRepositoryMetadata;
+import org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
@@ -50,7 +50,7 @@ public abstract class RepositoryConfigurationExtensionSupport implements Reposit
 	private static final String CLASS_LOADING_ERROR = "%s - Could not load type %s using class loader %s.";
 	private static final String MULTI_STORE_DROPPED = "Spring Data {} - Could not safely identify store assignment for repository candidate {}.";
 
-	protected static final String REPOSITORY_INTERFACE_POST_PROCESSOR = "org.springframework.data.repository.core.support.RepositoryInterfaceAwareBeanPostProcessor";
+	private static final String FACTORY_BEAN_TYPE_PREDICTING_POST_PROCESSOR = "org.springframework.data.repository.core.support.FactoryBeanTypePredictingBeanPostProcessor";
 
 	/* 
 	 * (non-Javadoc)
@@ -116,8 +116,15 @@ public abstract class RepositoryConfigurationExtensionSupport implements Reposit
 	 */
 	public void registerBeansForRoot(BeanDefinitionRegistry registry, RepositoryConfigurationSource configurationSource) {
 
-		registerIfNotAlreadyRegistered(new RootBeanDefinition(REPOSITORY_INTERFACE_POST_PROCESSOR), registry,
-				REPOSITORY_INTERFACE_POST_PROCESSOR, configurationSource.getSource());
+		String typeName = RepositoryFactoryBeanSupport.class.getName();
+
+		BeanDefinitionBuilder builder = BeanDefinitionBuilder
+				.rootBeanDefinition(FACTORY_BEAN_TYPE_PREDICTING_POST_PROCESSOR);
+		builder.addConstructorArgValue(typeName);
+		builder.addConstructorArgValue("repositoryInterface");
+
+		registerIfNotAlreadyRegistered(builder.getBeanDefinition(), registry, typeName.concat("_Predictor"),
+				configurationSource.getSource());
 	}
 
 	/**

src/main/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingBeanPostProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2014 the original author or authors.
+ * Copyright 2016 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.
@@ -15,6 +15,8 @@
  */
 package org.springframework.data.repository.core.support;
 
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -23,31 +25,56 @@ import org.slf4j.LoggerFactory;
 import org.springframework.beans.PropertyValue;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
+import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessorAdapter;
 import org.springframework.beans.factory.config.TypedStringValue;
 import org.springframework.core.Ordered;
 import org.springframework.core.PriorityOrdered;
+import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 /**
- * A {@link org.springframework.beans.factory.config.SmartInstantiationAwareBeanPostProcessor} implementing
- * {@code #predictBeanType(Class, String)} to return the configured repository interface from
- * {@link RepositoryFactoryBeanSupport}s. This is done as shortcut to prevent the need of instantiating
- * {@link RepositoryFactoryBeanSupport}s just to find out what repository interface they actually create.
+ * {@link InstantiationAwareBeanPostProcessorAdapter} to predict the bean type for {@link FactoryBean} implementations
+ * by interpreting a configured property of the {@link BeanDefinition} as type to be created eventually.
  * 
  * @author Oliver Gierke
+ * @since 1.12
+ * @soundtrack Ron Spielmann - Lock Me Up (Electric Tales)
  */
-class RepositoryInterfaceAwareBeanPostProcessor extends InstantiationAwareBeanPostProcessorAdapter implements
-		BeanFactoryAware, PriorityOrdered {
+public class FactoryBeanTypePredictingBeanPostProcessor extends InstantiationAwareBeanPostProcessorAdapter
+		implements BeanFactoryAware, PriorityOrdered {
 
-	private static final Logger LOGGER = LoggerFactory.getLogger(RepositoryInterfaceAwareBeanPostProcessor.class);
-	private static final Class<?> REPOSITORY_TYPE = RepositoryFactoryBeanSupport.class;
+	private static final Logger LOGGER = LoggerFactory.getLogger(FactoryBeanTypePredictingBeanPostProcessor.class);
 
 	private final Map<String, Class<?>> cache = new ConcurrentHashMap<String, Class<?>>();
+	private final Class<?> factoryBeanType;
+	private final List<String> properties;
 	private ConfigurableListableBeanFactory context;
 
+	/**
+	 * Creates a new {@link FactoryBeanTypePredictingBeanPostProcessor} predicting the type created by the
+	 * {@link FactoryBean} of the given type by inspecting the {@link BeanDefinition} and considering the value for the
+	 * given property as type to be created eventually.
+	 * 
+	 * @param factoryBeanType must not be {@literal null}.
+	 * @param properties must not be {@literal null} or empty.
+	 */
+	public FactoryBeanTypePredictingBeanPostProcessor(Class<?> factoryBeanType, String... properties) {
+
+		Assert.notNull(factoryBeanType, "FactoryBean type must not be null!");
+		Assert.isTrue(FactoryBean.class.isAssignableFrom(factoryBeanType), "Given type is not a FactoryBean type!");
+		Assert.notEmpty(properties, "Properties must not be empty!");
+
+		for (String property : properties) {
+			Assert.hasText(property, "Type property must not be null!");
+		}
+
+		this.factoryBeanType = factoryBeanType;
+		this.properties = Arrays.asList(properties);
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.beans.factory.BeanFactoryAware#setBeanFactory(org.springframework.beans.factory.BeanFactory)
@@ -66,7 +93,7 @@ class RepositoryInterfaceAwareBeanPostProcessor extends InstantiationAwareBeanPo
 	@Override
 	public Class<?> predictBeanType(Class<?> beanClass, String beanName) {
 
-		if (null == context || !REPOSITORY_TYPE.isAssignableFrom(beanClass)) {
+		if (null == context || !factoryBeanType.isAssignableFrom(beanClass)) {
 			return null;
 		}
 
@@ -77,24 +104,42 @@ class RepositoryInterfaceAwareBeanPostProcessor extends InstantiationAwareBeanPo
 		}
 
 		BeanDefinition definition = context.getBeanDefinition(beanName);
-		PropertyValue value = definition.getPropertyValues().getPropertyValue("repositoryInterface");
 
-		resolvedBeanClass = getClassForPropertyValue(value, beanName);
-		cache.put(beanName, resolvedBeanClass);
+		try {
+
+			for (String property : properties) {
+
+				PropertyValue value = definition.getPropertyValues().getPropertyValue(property);
+				resolvedBeanClass = getClassForPropertyValue(value, beanName);
 
-		return resolvedBeanClass == Void.class ? null : resolvedBeanClass;
+				if (Void.class.equals(resolvedBeanClass)) {
+					continue;
+				}
+
+				return resolvedBeanClass;
+			}
+
+			return null;
+
+		} finally {
+			cache.put(beanName, resolvedBeanClass);
+		}
 	}
 
 	/**
 	 * Returns the class which is configured in the given {@link PropertyValue}. In case it is not a
-	 * {@link TypedStringValue} or the value contained cannot be interpreted as {@link Class} it will return null.
+	 * {@link TypedStringValue} or the value contained cannot be interpreted as {@link Class} it will return {@link Void}.
 	 * 
-	 * @param propertyValue
-	 * @param beanName
+	 * @param propertyValue can be {@literal null}.
+	 * @param beanName must not be {@literal null}.
 	 * @return
 	 */
 	private Class<?> getClassForPropertyValue(PropertyValue propertyValue, String beanName) {
 
+		if (propertyValue == null) {
+			return Void.class;
+		}
+
 		Object value = propertyValue.getValue();
 		String className = null;
 
@@ -104,6 +149,16 @@ class RepositoryInterfaceAwareBeanPostProcessor extends InstantiationAwareBeanPo
 			className = (String) value;
 		} else if (value instanceof Class<?>) {
 			return (Class<?>) value;
+		} else if (value instanceof String[]) {
+
+			String[] values = (String[]) value;
+
+			if (values.length == 0) {
+				return Void.class;
+			} else {
+				className = values[0];
+			}
+
 		} else {
 			return Void.class;
 		}
@@ -111,8 +166,8 @@ class RepositoryInterfaceAwareBeanPostProcessor extends InstantiationAwareBeanPo
 		try {
 			return ClassUtils.resolveClassName(className, context.getBeanClassLoader());
 		} catch (IllegalArgumentException ex) {
-			LOGGER.warn(String.format("Couldn't load class %s referenced as repository interface in bean %s!", className,
-					beanName));
+			LOGGER.warn(
+					String.format("Couldn't load class %s referenced as repository interface in bean %s!", className, beanName));
 			return Void.class;
 		}
 	}

src/test/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingPostProcessorIntegrationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2013-2016 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.
@@ -20,26 +20,22 @@ import static org.junit.Assert.*;
 
 import java.util.Arrays;
 
-import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
-import org.springframework.core.SpringVersion;
 import org.springframework.data.querydsl.User;
 import org.springframework.data.repository.Repository;
 
 /**
  * Integration test to make sure Spring Data repository factory beans are found without the factories already
- * instantiated. Only executed for Spring version newer than 3.2.2.RELEASE.
+ * instantiated.
  * 
  * @see SPR-10517
  * @author Oliver Gierke
  */
-public class RepositoryInterfaceAwareBeanPostProcessorIntegrationTests {
-
-	static final String LAST_ERROR_VERSION = "3.2.2.RELEASE";
+public class FactoryBeanTypePredictingPostProcessorIntegrationTests {
 
 	DefaultListableBeanFactory factory;
 
@@ -54,7 +50,8 @@ public class RepositoryInterfaceAwareBeanPostProcessorIntegrationTests {
 		factory.registerBeanDefinition("repository", builder.getBeanDefinition());
 
 		// Register predicting BeanPostProcessor
-		RepositoryInterfaceAwareBeanPostProcessor processor = new RepositoryInterfaceAwareBeanPostProcessor();
+		FactoryBeanTypePredictingBeanPostProcessor processor = new FactoryBeanTypePredictingBeanPostProcessor(
+				RepositoryFactoryBeanSupport.class, "repositoryInterface");
 		processor.setBeanFactory(factory);
 		factory.addBeanPostProcessor(processor);
 	}
@@ -62,8 +59,6 @@ public class RepositoryInterfaceAwareBeanPostProcessorIntegrationTests {
 	@Test
 	public void lookupBeforeInstantiation() {
 
-		Assume.assumeTrue(isFixedSpringVersion());
-
 		String[] strings = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory, RepositoryFactoryInformation.class,
 				false, false);
 		assertThat(Arrays.asList(strings), hasItem("&repository"));
@@ -72,8 +67,6 @@ public class RepositoryInterfaceAwareBeanPostProcessorIntegrationTests {
 	@Test
 	public void lookupAfterInstantiation() {
 
-		Assume.assumeTrue(isFixedSpringVersion());
-
 		factory.getBean(UserRepository.class);
 
 		String[] strings = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory, RepositoryFactoryInformation.class,
@@ -81,11 +74,5 @@ public class RepositoryInterfaceAwareBeanPostProcessorIntegrationTests {
 		assertThat(Arrays.asList(strings), hasItem("&repository"));
 	}
 
-	private static boolean isFixedSpringVersion() {
-		return SpringVersion.getVersion().compareTo(LAST_ERROR_VERSION) == 1;
-	}
-
-	interface UserRepository extends Repository<User, Long> {
-
-	}
+	interface UserRepository extends Repository<User, Long> {}
 }

src/test/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingPostProcessorUnitTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2010 the original author or authors.
+ * Copyright 2008-2016 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
@@ -15,10 +15,12 @@
  */
 package org.springframework.data.repository.core.support;
 
-import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
+import java.io.Serializable;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -29,8 +31,7 @@ import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.data.repository.Repository;
-import org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport;
-import org.springframework.data.repository.core.support.RepositoryInterfaceAwareBeanPostProcessor;
+import org.springframework.jndi.JndiObjectFactoryBean;
 
 /**
  * Unit tests for {@link RepositoryInterfaceAwareBeanPostProcessor}.
@@ -38,29 +39,26 @@ import org.springframework.data.repository.core.support.RepositoryInterfaceAware
  * @author Oliver Gierke
  */
 @RunWith(MockitoJUnitRunner.class)
-public class RepositoryInterfaceAwareBeanPostProcessorUnitTests {
+public class FactoryBeanTypePredictingPostProcessorUnitTests {
 
 	private static final Class<?> FACTORY_CLASS = RepositoryFactoryBeanSupport.class;
 	private static final String BEAN_NAME = "foo";
 	private static final String DAO_INTERFACE_PROPERTY = "repositoryInterface";
 
-	private RepositoryInterfaceAwareBeanPostProcessor processor;
+	FactoryBeanTypePredictingBeanPostProcessor processor;
+	BeanDefinition beanDefinition;
 
-	@Mock
-	private ConfigurableListableBeanFactory beanFactory;
-	private BeanDefinition beanDefinition;
+	@Mock ConfigurableListableBeanFactory beanFactory;
 
 	@Before
 	public void setUp() {
 
-		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(FACTORY_CLASS).addPropertyValue(
-				DAO_INTERFACE_PROPERTY, UserDao.class);
+		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(FACTORY_CLASS)
+				.addPropertyValue(DAO_INTERFACE_PROPERTY, UserDao.class);
 		this.beanDefinition = builder.getBeanDefinition();
+		this.processor = new FactoryBeanTypePredictingBeanPostProcessor(FACTORY_CLASS, "repositoryInterface");
 
 		when(beanFactory.getBeanDefinition(BEAN_NAME)).thenReturn(beanDefinition);
-
-		processor = new RepositoryInterfaceAwareBeanPostProcessor();
-
 	}
 
 	@Test
@@ -80,8 +78,8 @@ public class RepositoryInterfaceAwareBeanPostProcessorUnitTests {
 	@Test
 	public void doesNotResolveInterfaceForUnloadableClass() throws Exception {
 
-		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(FACTORY_CLASS).addPropertyValue(
-				DAO_INTERFACE_PROPERTY, "com.acme.Foo");
+		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(FACTORY_CLASS)
+				.addPropertyValue(DAO_INTERFACE_PROPERTY, "com.acme.Foo");
 
 		when(beanFactory.getBeanDefinition(BEAN_NAME)).thenReturn(builder.getBeanDefinition());
 
@@ -97,16 +95,36 @@ public class RepositoryInterfaceAwareBeanPostProcessorUnitTests {
 		assertNotTypeDetected(FACTORY_CLASS);
 	}
 
-	private void assertNotTypeDetected(Class<?> beanClass) {
-
-		assertThat(processor.predictBeanType(beanClass, BEAN_NAME), is(nullValue()));
+	@Test(expected = IllegalArgumentException.class)
+	public void rejectsNonFactoryBeanType() {
+		new FactoryBeanTypePredictingBeanPostProcessor(Object.class, "property");
 	}
 
-	private class User {
+	/**
+	 * @see DATACMNS-821
+	 */
+	@Test
+	public void usesFirstValueIfPropertyIsOfArrayType() {
 
-	}
+		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(JndiObjectFactoryBean.class);
+		builder.addPropertyValue("proxyInterfaces",
+				new String[] { Serializable.class.getName(), Iterable.class.getName() });
 
-	private interface UserDao extends Repository<User, Long> {
+		when(beanFactory.getBeanDefinition(BEAN_NAME)).thenReturn(builder.getBeanDefinition());
 
+		processor = new FactoryBeanTypePredictingBeanPostProcessor(JndiObjectFactoryBean.class, "proxyInterface",
+				"proxyInterfaces");
+		processor.setBeanFactory(beanFactory);
+
+		assertThat(processor.predictBeanType(JndiObjectFactoryBean.class, BEAN_NAME),
+				is(typeCompatibleWith(Serializable.class)));
+	}
+
+	private void assertNotTypeDetected(Class<?> beanClass) {
+		assertThat(processor.predictBeanType(beanClass, BEAN_NAME), is(nullValue()));
 	}
+
+	private class User {}
+
+	private interface UserDao extends Repository<User, Long> {}
 }