Osvaldo Pina opened SPR-16694 and commented
The problem appears when: 1-The bean and bean factory are registered through an ImportBeanDefinitionRegistrar 2-A bean is created through a factory that returns a bean supertype 3-The reference required to be injected is any subtype of factory method return 4-The bean instance is not created when the injection search happens
Upon inspecting what was happening, I discovered that the AbstractAutowireCapableBeanFactory.determineTargetType method sets the resolvedTargetType to be the value returned by the bean factory, without considering that the actual bean type could be a subtype of it.
protected Class<?> determineTargetType(String beanName, RootBeanDefinition mbd, Class<?>... typesToMatch) {
Class<?> targetType = mbd.getTargetType();
if (targetType == null) {
targetType = (mbd.getFactoryMethodName() != null ?
getTypeForFactoryMethod(beanName, mbd, typesToMatch) :
resolveBeanClass(mbd, beanName, typesToMatch));
if (ObjectUtils.isEmpty(typesToMatch) || getTempClassLoader() == null) {
mbd.resolvedTargetType = targetType;
}
}
return targetType;
}
The problem only occurs if the bean search occurs before the bean instance is created because the AbstractAutowireCapableBeanFactory.doCreateBean sets the resolvedTargetType as the actual instance type.
...
if (beanType != NullBean.class) {
mbd.resolvedTargetType = beanType;
}
...
if this is actually confirmed as a bug a possible solution would be to consider both the bean class and the factory class in determining the bean's final class
protected Class<?> determineTargetType(String beanName, RootBeanDefinition mbd, Class<?>... typesToMatch) {
Class<?> targetType = mbd.getTargetType();
if (targetType == null) {
targetType = getBeanType(beanName,
resolveBeanClass(mbd, beanName, typesToMatch),
mbd.getFactoryMethodName() != null ? getTypeForFactoryMethod(beanName, mbd, typesToMatch) : null);
if (ObjectUtils.isEmpty(typesToMatch) || getTempClassLoader() == null) {
mbd.resolvedTargetType = targetType;
}
}
return targetType;
}
private Class<?> getBeanType(String beanName, Class<?> beanType, Class<?> beanFactoryType) {
if (beanType == null && beanFactoryType == null) {
return null;
}
else if (beanType != null && beanFactoryType != null) {
if (!beanType.isAssignableFrom(beanFactoryType) && !beanFactoryType.isAssignableFrom(beanType)) {
throw new BeanCreationException(beanName, " bean type [" + beanType.getName() +
"] that is incompatible with bean factory type [" + beanFactoryType + "}");
}
else {
return beanType;
}
}
else {
return beanType != null ? beanType : beanFactoryType;
}
}
Threre are 2 unit tests atteached that shows the problem
Affects: 4.3.15, 5.0.4
Attachments: - bug-spring.zip (16.91 kB)
Comment From: spring-projects-issues
Juergen Hoeller commented
It looks like we cannot reliably determine that type at that point: Until we actually call the factory method, we don't know the concrete type of the bean... and we can't make assumptions about the declaring class either since a factory method may be declared on a completely different factory class, with factory methods returning the same type that they are declared on being a special case only.
So I'm afraid we can't make assumptions about it in injection matching either. We generally recommend for factory method return type declarations to be as specific as possible, and for injection point types to be as abstract as possible.
If you're declaring those factory methods on the specific type anyway, couldn't you simply narrow their return type to that specific type as well? That would immediately allow for reliable assumptions on the core container's side.
Comment From: spring-projects-issues
Osvaldo Pina commented
I agree that only after the actually bean instance creation the type can be determined but before that the most accurate information wouldn't be the bean type as defined in the registered bean configuration? A caveat would be a situation when factory method return type and bean type don't belong to the same type hierarchy, but that wold be an error.
I think the rule the to determine the bean type would could be summarized in the following table:
||bean factory return type || registered bean type || type to be considered on injection | a type | a type | a type | | parent | child | child | | child | parent | parent | | a type | another type | bean creation exception |
The idea would be to assume the developer intention when he/she sets the bean definition type until the limit of type compatibility between bean type and factory type. The factory would be just a mean to create beans and not the most reliable type source.
I think also that after bean creation, if resolvedTargetType was already set, another type check should be made to guarantee that what was used as bean type was respected at instance creation. But I didn't studied the bean creation code to have a better understanding of possible side effects...
Comment From: spring-projects-issues
Juergen Hoeller commented
The problem here is that the configured bean class is technically just the containing class for the factory method and does not imply any hints about its return type. See for example @Configuration classes where @Bean methods return specific bean instances that have no type relationship with the containing configuration class.
We could theoretically check whether the containing class is a subtype of the declared factory method return type... but that would be a rather specific new assumption. Even then it's not guaranteed - and not well-defined anywhere - that the factory method would have to actually return an instance of that containing subtype.
Once again, I wonder why such a specific factory method couldn't simply declare the specific subtype as its return type? The problem would go away then, wouldn't it?
Comment From: spring-projects-issues
Osvaldo Pina commented
You have asked me this in the last two messages .. apologies. The factory would be generic and would create beans of different types depending on some settings.
Comment From: spring-projects-issues
Osvaldo Pina commented
The scenario simplified version is:
We have a framework that creates what we call "business components". Each business component has its own jar module and is accessed via a interface called "bussiness interface".
What I'm trying to do is to inspect the runtime configuration (a collection of property files) and determine all possible bussiness componentes. After that I would check which business interfaces classes are avaliable and for each one create the correspondent bean trought a generic factory. That way I think we can solve the problem with just one more module.
The alternatives that I can see is to embedd a factory in each business component or create a separete module with a factory for each business components. The problem is that thre are hundred of them
Comment From: snicoll
Declaring the most specific type is really what you should be doing. Even more so with modern Spring where that information can be used to optimize an application context ahead of time.