From deb884dfb16badd99de6a4a5901185df225fa967 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 24 Feb 2016 19:20:43 +0100 Subject: [PATCH] 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. --- ...positoryConfigurationExtensionSupport.java | 17 +++- ...yBeanTypePredictingBeanPostProcessor.java} | 93 +++++++++++++++---- ...dictingPostProcessorIntegrationTests.java} | 25 ++--- ...TypePredictingPostProcessorUnitTests.java} | 64 ++++++++----- 4 files changed, 133 insertions(+), 66 deletions(-) rename src/main/java/org/springframework/data/repository/core/support/{RepositoryInterfaceAwareBeanPostProcessor.java => FactoryBeanTypePredictingBeanPostProcessor.java} (54%) rename src/test/java/org/springframework/data/repository/core/support/{RepositoryInterfaceAwareBeanPostProcessorIntegrationTests.java => FactoryBeanTypePredictingPostProcessorIntegrationTests.java} (76%) rename src/test/java/org/springframework/data/repository/core/support/{RepositoryInterfaceAwareBeanPostProcessorUnitTests.java => FactoryBeanTypePredictingPostProcessorUnitTests.java} (63%) diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupport.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupport.java index a7c83033e..c94406d73 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtensionSupport.java +++ b/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()); } /** diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessor.java b/src/main/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingBeanPostProcessor.java similarity index 54% rename from src/main/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessor.java rename to src/main/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingBeanPostProcessor.java index b42e51aa2..b2afe0574 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessor.java +++ b/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> cache = new ConcurrentHashMap>(); + private final Class factoryBeanType; + private final List 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 { - return resolvedBeanClass == Void.class ? null : resolvedBeanClass; + for (String property : properties) { + + PropertyValue value = definition.getPropertyValues().getPropertyValue(property); + resolvedBeanClass = getClassForPropertyValue(value, beanName); + + 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; } } diff --git a/src/test/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessorIntegrationTests.java b/src/test/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingPostProcessorIntegrationTests.java similarity index 76% rename from src/test/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessorIntegrationTests.java rename to src/test/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingPostProcessorIntegrationTests.java index b449d7a6f..49f852fb8 100644 --- a/src/test/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessorIntegrationTests.java +++ b/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 { - - } + interface UserRepository extends Repository {} } diff --git a/src/test/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessorUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingPostProcessorUnitTests.java similarity index 63% rename from src/test/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessorUnitTests.java rename to src/test/java/org/springframework/data/repository/core/support/FactoryBeanTypePredictingPostProcessorUnitTests.java index d8cfdbeee..2ee1da565 100644 --- a/src/test/java/org/springframework/data/repository/core/support/RepositoryInterfaceAwareBeanPostProcessorUnitTests.java +++ b/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) { + @Test(expected = IllegalArgumentException.class) + public void rejectsNonFactoryBeanType() { + new FactoryBeanTypePredictingBeanPostProcessor(Object.class, "property"); + } + /** + * @see DATACMNS-821 + */ + @Test + public void usesFirstValueIfPropertyIsOfArrayType() { + + BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(JndiObjectFactoryBean.class); + builder.addPropertyValue("proxyInterfaces", + new String[] { Serializable.class.getName(), Iterable.class.getName() }); + + 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 class User {} - } - - private interface UserDao extends Repository { - - } + private interface UserDao extends Repository {} }