JavaConfig Bean overriding with addition [SPR-10988]

[spring-projects-issues]

Benoit Lacelle opened SPR-10988 and commented

I consider injection of beans as a List of automatically detected beans: I introduce several beans implementing the same interface and inject all of them as a List in a later bean.

I've not been able to find official documentation related to this feature. My single source is http://www.coderanch.com/t/605509/Spring/Java-config-autowired-List

Consider this feature, I have an issue with Bean overring: I would like to override a bean defined throught a no-arg method with a bean defined with the List of detected beans. However, spring behave like the second bean definition does not exist.

It can be reproduced with the following test:

import java.util.Date;
import java.util.List;

import org.junit.Assert;
import org.junit.Test;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

public class SpringTest {

	@Test
	public void shouldtestSpringDifferentMethodNames() {
		AnnotationConfigApplicationContext ctx2 = new AnnotationConfigApplicationContext(AConfig.class, CConfig.class);
		Assert.assertEquals("overriden", ctx2.getBean("bean"));
	}

	@Configuration
	public static class AConfig {

		@Bean
		public Object bean() {
			return "not overriden";
		}

	}

	@Configuration
	public static class CConfig extends AConfig {

		@Bean
		public Date anotherBean() {
			return new Date();
		}

		@Bean
		public Object bean(List<? extends Date> someDate) {
			return "overriden";
		}

	}

}

If this is an expected behavior, how can I achieve such an overriding?

I asked the same question on SOF: http://stackoverflow.com/questions/19377789/javaconfig-bean-overriding-failing-with-list-injected

Thanks

---

Affects: 3.2.3

Issue Links:

• ObjectProvider iterable/stream access for "beans of type" resolution in @Bean methods [SPR-11419] #16046 ObjectProvider iterable/stream access for "beans of type" resolution in @Bean methods
• ConfigurationClass.validate() should allow for overloading in general or not at all [SPR-11025] #15653 ConfigurationClass.validate() should allow for overloading in general or not at all

Referenced from: commits b093b84, b00c31a, 78c10cd

0 votes, 5 watchers

[spring-projects-issues]

Phil Webb commented

I am not really sure what you are asking here. Could you try to explain in a little more detail. Overriding @Bean methods on @Configuration classes is somewhat unusual and it is not clear to me what you are trying to achieve by doing that.

[spring-projects-issues]

Benoit Lacelle commented

Considering a library providing some @Configuration classes, I need to override one of these beans. The new bean has a constructor dependency, which is the list of beans of a given type. So the bean definitions changed from

@Bean
public Number bean() {
    return new Integer(3);
}

to

@Bean
public Number bean(List<? extends Date> someDate) {
    return new Double(someDate.size());
}

By "Overriding @Bean methods on @Configuration classes is somewhat unusual", do you mean there is an alternative way which is more common? I'm used to XML configuration and I'm trying to achieve the equivalent of bean overriding by re-using the name of an existing bean.

I get the expected behavior if CCOnfig does not extends AConfig. Would this be a moreusual way of overriding? I tend to extends @Configuration classes for overridings as, in some cases, the overriding is enforced through an @Override annotation.

[spring-projects-issues]

Jose Luis Martin commented

I guess that problem is in ConfigurationClass.validate() method.

The Configuration class obtained by inheritance (CConfig) has two overloaded methods, bean() and bean(List date). So validate() should report a BeanMethodOverloadingProblem and throw a BeanDefinitionParsingException.

I tried to fix it replacing the code that checks for overloaded methods in validate() and seems to work, but MethodMetadata interface don't give access to the introspected method and there are dirty access to it casting to StandardMethodMetadata class.

// An @Bean method may only be overloaded through inheritance. No single
// @Configuration class may declare two @Bean methods with the same name.
Map<String, List<BeanMethod>> methods = new HashMap<String, List<BeanMethod>>();
for (BeanMethod beanMethod : this.beanMethods) {
	String methodName = beanMethod.getMetadata().getMethodName();
	List<BeanMethod> methodList;
	if (!methods.containsKey(methodName)) {
		methodList = new ArrayList<BeanMethod>();
		methods.put(methodName, methodList);
	}
	else {
		methodList = methods.get(methodName);
	}
	methodList.add(beanMethod);
}

for (String methodName : methods.keySet()) {
	List<BeanMethod> methodList = methods.get(methodName);
	if (methodList.size() > 1) {
		// Check for overloading
		Class<?>[] firstParameters =
				((StandardMethodMetadata) methodList.get(0).getMetadata())
				.getIntrospectedMethod().getParameterTypes();
			for (BeanMethod method : methodList) {
			Class<?>[] methodParameters = ((StandardMethodMetadata) method.getMetadata())
					.getIntrospectedMethod().getParameterTypes();
				if (!Arrays.equals(firstParameters, methodParameters)) {
				// Overloaded
				problemReporter.error(new BeanMethodOverloadingProblem(methodName, methodList.size()));
			}
		}
	}
}

Full commit is in chelu@c4e6895

[spring-projects-issues]

Phil Webb commented

Benoit Lacelle,

Thanks for the information. That makes much more sense now. When I said:

Overriding @Bean methods on @Configuration classes is somewhat unusual

I was thinking in the context of a single application, as you are talking about reuse of a common configuration provided by a shared library I can see why you would do this. In fact we do this ourselves with classes such as WebMvcConfigurationSupport.

As Jose Luis Martin points out, the restriction on overloaded methods is intentional.

One option that might work for you is to leave the @Bean method as-is and autowire the detected beans to a private member:

@Configuration
public static class CConfig extends AConfig {

	@Autowired
	private List<Date> dateBeans;

	@Bean
	public Date anotherBean() {
		return new Date();
	}

	@Bean
	public Object bean() {
		// can access this.dateBeans here
		return "overriden";
	}
}

You could also make CConfig implement ApplicationContextAware then use the methods from BeanFactoryUtils to get what you need.

[spring-projects-issues]

Benoit Lacelle commented

Thanks for the details Phil.

I'm not sure to understand one point: is it legit for a @Configuration to override a bean from a parent @Configuration? I understand it is legit given WebMvcConfigurationSupport but not legit as "the restriction on overloaded methods is intentional". Is it legit only in some cases?

Anyway, if the override describe in this ticket is not legit, a fail-fast fix like the one from Jose Luis Martin would be much helpful.

[spring-projects-issues]

Phil Webb commented

It is legal to override @Bean methods but you cannot overload them. So you can override the method, but not create a new version with additional arguments.

[spring-projects-issues]

Jose Luis Martin commented

Hi Phil

I agree that work as designed but ConfigurationClass.validate() still is accepting a invalid configuration.

As this bug is now Resolved, should I report a new bug for that?

[spring-projects-issues]

Phil Webb commented

Jose Luis Martin, I think this one is getting a little cluttered so yes please, a new bug would be appreciated.

[spring-projects-issues]

Benoit Lacelle commented

Thanks for your help.

I was convinced bean overriding through method overloading was legal. Fact is the following overload works:

public class SpringTest {

	@Test
	public void shouldtestSpringDifferentMethodNames() {
		AnnotationConfigApplicationContext ctx2 = new AnnotationConfigApplicationContext(AConfig.class, CConfig.class);
		Assert.assertEquals("overriden", ctx2.getBean("bean"));
	}

	@Configuration
	public static class AConfig {

		@Bean
		public Object bean() {
			return "not overriden";
		}

	}

	@Configuration
	public static class CConfig extends AConfig {

		@Bean
		public Date anotherBean() {
			return new Date();
		}

		@Bean
		public Object bean(Date someDate) {
			return "overriden";
		}

	}

}

This is why I was confused about which bean overridings are legal and which ones are not.

Jose,
I believe your fix will make fail the test described in this comment. Then, I tend to believe it shall break existing configurations.

Should we take in account @Bean(override=true) to allow bean overloading?

[spring-projects-issues]

Phil Webb commented

On further investigation is appears that overloaded bean methods are intended to work the way that you describe. BeanMethodPolymorphismTests shows an example. It appears that your specific issue relates to the fact that you are injecting a List of beans, as opposed to a single item.

It seems that the real problem may lie in the ConstructorResolver class (and specifically the getTypeDifferenceWeight method) which is weighing the no-arg method above the version that accepts a List.

[spring-projects-issues]

Benoit Lacelle commented

Great Phil. Would you consider porting the fix in the 3.2 branch?

[spring-projects-issues]

Phil Webb commented

Unfortunately I don't yet have fix. I am going to assign this one to Juergen Hoeller to take a look at.

[spring-projects-issues]

Jose Luis Martin commented

I see... though I never would expect that a class will work differently depending on whether it was obtained through inheritance or not. (!)

The List argument is not the only problem. As methods are ordered using AutowireUtils.sortConstructors(), overloading a bean method with a less arguments one will fail too.

I think that this issue is related to #15620 because overloaded bean definitions are overriden by superclass too.

[spring-projects-issues]

Jose Luis Martin commented

I'm a bit confused here because overloaded/overwriten methods are supposed to be supported by configuration classes but I have not found any code that handle it in the framework.

I have done a new pull request trying add support for both features following the indications of javadoc of BeanMethodPolymorphismTests, ie choosing the most specifc method in the hierarchy.

Instead of dealing with ConstructorResolver, I chose to assign the factory method to use at parsing time.

The pull request has merged the changes that I proposed to #15620 because overloaded bean definitions are overriden by superclasses too.

[spring-projects-issues]

Phil Webb commented

See also this comment from #15653.