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

Overloaded @Bean method with name mismatch causes bean to be created twice (in case of ASM processing) #25263

Closed
jnizet opened this issue Jun 17, 2020 · 10 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@jnizet
Copy link

jnizet commented Jun 17, 2020

Reproduced in Spring 5.2.7.RELEASE (Spring Boot 2.3.1.RELEASE):

@Configuration
public class SomeConfig {
    @Bean(name = "other")
    SomeOtherBean foo() {
        System.out.println("constructing SomeOtherBean");
        return new SomeOtherBean();
    }

    @Bean(name = "foo")
    SomeBean foo(@Qualifier("other") SomeOtherBean other) {
        System.out.println("constructing SomeBean");
        return new SomeBean(other);
    }
}

With the above configuration class, SomeOtherBean is constructed twice, and SomeBean is never constructed. Why? Because the two methods have the same name. If you rename the second method to bar, everything is constructed once, as expected.
Admittedly, using the same method name is not a good idea. I just happened to have done it by accident.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 17, 2020
@lifejwang11
Copy link

Reproduced in Spring 5.2.7.RELEASE (Spring Boot 2.3.1.RELEASE):

@Configuration
public class SomeConfig {
    @Bean(name = "other")
    SomeOtherBean foo() {
        System.out.println("constructing SomeOtherBean");
        return new SomeOtherBean();
    }

    @Bean(name = "foo")
    SomeBean foo(@Qualifier("other") SomeOtherBean other) {
        System.out.println("constructing SomeBean");
        return new SomeBean(other);
    }
}

With the above configuration class, SomeOtherBean is constructed twice, and SomeBean is never constructed. Why? Because the two methods have the same name. If you rename the second method to bar, everything is constructed once, as expected.
Admittedly, using the same method name is not a good idea. I just happened to have done it by accident.

hello,I think you might need to provide a demo,Because I don't know the relationship between the two,Thanks.

@jnizet
Copy link
Author

jnizet commented Jun 17, 2020

@lifejwang11 not sure exactly what you mean by "the relationship between the two". If you're talking about the two beans SomeBean and SomeOtherBean, they're irrelevant. Spring should construct an instance of each of them by calling each of the @Bean methods once, but does not, and instead calls the first one twice, and never calls the second one.

But anyway, here's a complete demo if you want to try it by yourself: https://github.com/jnizet/spring-overload-issue

@sbrannen
Copy link
Member

sbrannen commented Jun 17, 2020

If I execute the following test against master ...

@SpringJUnitConfig
class SomeConfigTests {

	@Test
	void test() {
	}

	@Configuration
	static class SomeConfig {

		@Bean(name = "other")
		public SomeOtherBean foo() {
			System.out.println("constructing SomeOtherBean");
			return new SomeOtherBean();
		}

		@Bean(name = "foo")
		public SomeBean foo(@Qualifier("other") SomeOtherBean other) {
			System.out.println("constructing SomeBean");
			return new SomeBean(other);
		}
	}
}

The output is:

constructing SomeOtherBean
constructing SomeBean

And the output is the same for the 5.2.x branch as well as the v5.2.7.RELEASE release tag.


Though, I can confirm that the output from your sample Spring Boot application is the following.

constructing SomeOtherBean
constructing SomeOtherBean

@sbrannen
Copy link
Member

Since I have not been able to reproduce this in Spring Framework 5.2.x or master, I'm wondering if the behavior is specific to Spring Boot.

@philwebb and @snicoll, would you be willing to look at this from the perspective of Spring Boot?

@wilkinsona
Copy link
Member

The unit test isn't functionally equivalent to the sample as you have removed component scanning from the equation. The sample application still reproduces the problem when modified to remove any use of Spring Boot:

package com.example.demo;

import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.ComponentScan;

@ComponentScan
public class DemoApplication {

    public static void main(String[] args) {
    	new AnnotationConfigApplicationContext(DemoApplication.class);
    }

}

@sbrannen sbrannen added this to the 5.3 M2 milestone Jun 25, 2020
@sbrannen sbrannen self-assigned this Jun 25, 2020
@sbrannen
Copy link
Member

Tentatively slated for 5.3 M2 for further investigation

@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Jun 26, 2020
@lifejwang11
Copy link

Tentatively slated for 5.3 M2 for further investigation

I think the key to this question is AbstractAutowireCapableBeanFactory#getTypeForFactoryMethod

	for (Method candidate : candidates) {
				if (Modifier.isStatic(candidate.getModifiers()) == isStatic && mbd.isFactoryMethod(candidate) &&
						candidate.getParameterCount() >= minNrOfArgs) {
					// doSmonething
				}
			}

If you have two methods with the same name and bean annotation in the same class,I think you need to determine if beanName is the same,so I think this code should be modified in ConfigurationClassBeanDefinitionReader#isFactoryMethod

old

@Override
		public boolean isFactoryMethod(Method candidate) {
			return (super.isFactoryMethod(candidate) && BeanAnnotationHelper.isBeanAnnotated(candidate));
		}

new

		@Override
		public boolean isFactoryMethod(Method candidate) {
			return (super.isFactoryMethod(candidate) && BeanAnnotationHelper.isBeanAnnotated(candidate) && BeanAnnotationHelper.determineBeanNameFor(candidate).equals(candidate.getName()));
		}

If my idea is right, I would like to submit a PR,thanks

@lifejwang11
Copy link

@sbrannen The bug I think Methods with @bean should not only determine if the method name is the same, but also if the name attribute corresponding to the bean annotation of the method (or the default method name without the name attribute) is the same as the current method name,I think the way to do bean annotation is like this, don't you think

lifejwang11 pushed a commit to lifejwang11/spring-framework that referenced this issue Jun 30, 2020
This was referenced Jun 30, 2020
@kse-music
Copy link

in spring framework 5.2.8 also exist this issue

@sbrannen sbrannen modified the milestones: 5.3 M2, 5.3 RC1 Aug 10, 2020
@jhoeller jhoeller assigned jhoeller and unassigned sbrannen Aug 24, 2020
@jhoeller jhoeller added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 27, 2020
@jhoeller jhoeller modified the milestones: 5.3 RC1, 5.2.9 Aug 27, 2020
@jhoeller
Copy link
Contributor

I've tracked this down to a difference between Class-based processing of the configuration class versus ASM-based processing against a registered configuration class name. A variant of the unit test passes with Class registration (similar to the @SpringJUnitConfig scenario above) but fails with name-based registration (similar to component scanning).

@jhoeller jhoeller changed the title Overloaded @Bean method causes bean to be created twice and other bean not to be created Overloaded @Bean method with name mismatch causes bean to be created twice (in case of ASM processing) Aug 27, 2020
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) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

7 participants