Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected @Bean Definition Behavior when Using Inheritance in @Configuration Classes #28137

Closed
mbach979 opened this issue Mar 4, 2022 · 1 comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue

Comments

@mbach979
Copy link

mbach979 commented Mar 4, 2022

Affects: 5.3.16


The interaction of Spring @configuration annotated classes with @bean annotated method and inheritance behaves sort of like normal java inheritance and sort of like inclusion, but not consistently in either way.

For example if:

  1. a parent class has an @bean method and
  2. the child class overrides that method, and
  3. then only the child class is registered/imported by the context

and given:

  • the normal java rules of annotation inheritance on methods i.e. they are not inherited by overridden methods

one would expect:

  1. the bean defined by the parent method to not be defined if the child does not re-annotated the method with an @bean
  2. or, the bean defined by the parent method should be renamed if the method is re-annotated with a @bean with a different name
  3. or, the bean defined by the parent method should be overridden if the method is re-annotated with a @bean with no name/or the same name.

Expectations 1 and 2 are not met in that the bean defined by the parent class does exist in the context. And expectation 3 is only met if the names agree between the child and parent.

One could maybe argue that inheritance behaves like inclusion, which I would find weird, but a model, but then that is not consistent with itself, in that the value of the bean will be the result of the overridden method and not the parent method.

Other people have also been confused by this: https://stackoverflow.com/questions/39573558/spring-java-config-extending-an-abstract-configuration. The answers in that SO post are not really satisfying and amount to "well that's how it's implemented".

The following code reproduces the issue (also attached as a gradle project):

public class SpringConfigInheritance {

	@Configuration
	public static class ParentConfig {
		@Bean
		public String bean() {
			return "parent";
		}
		
		@Bean
		public String configClass() {
			return getClass().getName();
		}
	}
	
	@Configuration
	public static class ChildConfig extends ParentConfig {
		@Bean("renamed-bean")
		@Override
		public String bean() {
			return "child";
		}
	}
	
	public static void main(String[] args) throws IllegalAccessException, IllegalArgumentException, InvocationTargetException {
	
		System.out.println("Given normal inheritance rules and the rules for annotations on methods the expected output would be:");
	
		ChildConfig childConfig = new ChildConfig();
		for ( Method method : ChildConfig.class.getMethods() ) {
			if ( method.isAnnotationPresent(Bean.class) ) {
				String[] names = method.getAnnotation(Bean.class).value();
				Object   value = method.invoke(childConfig);
				System.out.println("@Bean method: " + method.getName() + 
								   "\n\tBean names: " + Arrays.stream(names).collect(Collectors.joining(", ")) +
								   "\n\tBean value: " + Objects.toString(value));
			}
		}
		
		System.out.println();
		System.out.println("What spring gives us is actually: ");
		
		// Parent with one child config
		try ( AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext() ) {
			context.register(ChildConfig.class);
			context.refresh();
			
			for ( String name : context.getBeanDefinitionNames() ) {
				BeanDefinition definition = context.getBeanDefinition(name);
				if ( definition.getRole() == BeanDefinition.ROLE_APPLICATION && definition.getFactoryMethodName() != null)
				{
					Object value = context.getBean(name);
					
					System.out.println("@Bean method: " + definition.getFactoryMethodName() + 
								   "\n\tBean names: " + name +", " + Arrays.stream(context.getAliases(name)).collect(Collectors.joining(", ")) +
								   "\n\tBean value: " + Objects.toString(value));				
				}
			}
			
			System.out.println();
			System.out.println("Let's see if this is consistent with an inheritence == inclusion model by running some tests:");

			// Why does this bean exist?  We only included ChildConfig.
			// Okay, so inheritance == inclusion?  Maybe this make sense because of parallels to XML config?
			boolean hasParentBean = context.containsBean("bean");
			System.out.printf("parent bean existence: %s -> inheritance %s inclusion\n", hasParentBean, hasParentBean ?"==":"!=");
			
			if ( hasParentBean ) {
				// Let's double check that it isn't just an alias now, which would be consistent with inclusion
				boolean isAlias = context.isAlias("bean");
				System.out.printf("parent bean is alias test: %s -> inheritance %s inclusion\n", isAlias, isAlias?"!=":"==");
				
				// Let's double check that it isn't just an alias now, which would be consistent with inclusion
				isAlias = context.isAlias("renamed-bean");
				System.out.printf("child bean is alias test: %s -> inheritance %s inclusion\n", isAlias, isAlias?"!=":"==");
				
				// Alright, so if inheritance == inclusion, then the value should be "parent" 
				String parentBean = context.getBean("bean", String.class);
				boolean isExpectedValueViaInclusion = parentBean.equals("parent");
				System.out.printf("parent bean value == \"parent\" test: %s -> inheritance %s inclusion\n", isExpectedValueViaInclusion, isExpectedValueViaInclusion?"==":"!=");
				
				// Alright, so if inheritance == inclusion, then the configClass value should be "ParentConfig"???
				String configClass = context.getBean("configClass", String.class);
				boolean isExpectedConfigClassViaInclusion = configClass.equals("springBugs.configInheritance.SpringConfigInheritance$ParentConfig");
				System.out.printf("parent configClass value == \"springBugs.configInheritance.SpringConfigInheritance$ParentConfig\" test: %s -> inheritance %s inclusion\n", isExpectedConfigClassViaInclusion, isExpectedConfigClassViaInclusion?"==":"!=");
			}
		}
	}
}

Running this program produces the following output:

Given normal inheritance rules and the rules for annotations on methods the expected output would be:

@Bean method: bean
	Bean names: renamed-bean
	Bean value: child
@Bean method: configClass
	Bean names: 
	Bean value: springBugs.configInheritance.SpringConfigInheritance$ChildConfig

What spring gives us is actually:

@Bean method: bean
	Bean names: renamed-bean, 
	Bean value: child
@Bean method: bean
	Bean names: bean, 
	Bean value: child
@Bean method: configClass
	Bean names: configClass, 
	Bean value: springBugs.configInheritance.SpringConfigInheritance$ChildConfig$$EnhancerBySpringCGLIB$$cb54331c

Let's see if this is consistent with an inheritence == inclusion model by running some tests:

parent bean existence: true -> inheritance == inclusion
parent bean is alias test: false -> inheritance == inclusion
child bean is alias test: false -> inheritance == inclusion
parent bean value == "parent" test: false -> inheritance != inclusion
parent configClass value == "springBugs.configInheritance.SpringConfigInheritance$ParentConfig" test: false -> inheritance != inclusion

I believe the correct behavior, as outlined above, should be consistent with Java method annotation rules w/r/t inheritance.

I'm not exactly sure I see the exact fix, but it would seem that ConfigurationClassParser should do some checking of overridden @bean methods prior to adding them to the ConfigurationClass. Since they are recursively parsed, starting at the actually registered class, it should be possible to do a check for overloads prior to registering. Or maybe this is the wrong place altogether to do it.

At a minimum, I think that the documentation should be clarified about what the rules and effects are of inheritance in @configuration annotated classes.

spring-container-inheritance-bug.tar.gz

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 4, 2022
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Mar 5, 2022
@snicoll
Copy link
Member

snicoll commented Dec 12, 2023

Duplicate #28286

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Dec 12, 2023
@snicoll snicoll added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

4 participants