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

Map injection fails to find bean produced with @Bean when additional method with same name exists [SPR-16999] #21537

Closed
spring-issuemaster opened this issue Jul 3, 2018 · 4 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Jul 3, 2018

Kai Burjack opened SPR-16999 and commented

I wanted to use the Map<String, T> beans injection to quickly iterate over all beans implementing a given type and also to index into a bean via its name. However, we just identified a case that does not work (but probably should).

The case is, when a @Configuration class including a @Bean factory method also declares another (non-@Bean) method with the same name (but of course different parameter types) then the Map injection won't find the bean defined by the @Bean method.

Here is a test case: 

@Configuration
public class Test {
  @Autowired
  private Map<String, Runnable> testBeans;
  @Bean
  Runnable testBean() {
    return () -> {};
  }
  private void testBean(boolean param) {
  }
  public static void main(String[] args) {
    new AnnotationConfigApplicationContext(Test.class);
  }
}

This fails with injection failure of "testBeans" because Spring could not find any beans implementing Runnable. That, however, is not true. Since when removing the Map property and instead performing a manual lookup on the context via getBean("testBean") or even getBean(Runnable.class), it will find the bean.

 When removing the private void testBean(boolean) method, Map injection works again.


Affects: 4.3.18, 5.0.7

Backported to: 4.3.19

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 3, 2018

Juergen Hoeller commented

Good catch! Our algorithm for introspecting factory method return types did not filter actual @Bean methods but was rather looking at all methods of the same name, like for a plain factory-method = "testBean" declaration in XML. I've brought it in sync with our constructor resolution algorithm now, filtering for actual factory method candidates the same way.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 3, 2018

Kai Burjack commented

Hi! Thanks. I was thinking that the Map<String, T> injection mechanism simply calls into ApplicationContext.ggetBeansOfType() to obtain all beans of a given type (asked for by the value type parameter of the Map) instead of actually using a custom algorithm to resolve the beans...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 3, 2018

Juergen Hoeller commented

It actually calls into AutowireCapableBeanFactory.resolveDependency which delegates to findAutowireCandidates... and that's where it joins getBeanNamesForType which is also what getBeansOfType is calling. The type prediction algorithm is the same at that level. The main difference is whether the bean exists at the time of the type check already or whether we have to do a pre-instantiation type guess... which is where the factory method filtering is duplicated.

As a consequence, this prediction mismatch existed in both code paths and should be fixed for good for all those variants now.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 3, 2018

Kai Burjack commented

Alright, thank you for the explanation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.