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

Inconsistent behavior when autowiring multiple beans by type as collection without matching beans #22735

Closed
therob opened this issue Apr 4, 2019 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: documentation A documentation task
Milestone

Comments

@therob
Copy link

therob commented Apr 4, 2019

Affects: 5.1.0 and later

Autowiring multiple beans as a collection results in different behavior depending on the type of injection used when no matching beans are found.

Using field injection or setter injection leads to an UnsatisfiedDependencyException. Whereas constructor injection results in an empty collection. According to the docs this should fail too.

1.9.2. Using @Autowired

By default, the autowiring fails whenever zero candidate beans are available. The default behavior is to treat annotated methods, constructors, and fields as indicating required dependencies.

A demo of this can be found here: https://github.com/therob/bug-demo-inject-collection
Just execute the included test with mvn test or your favourite IDE.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 4, 2019
@jhoeller
Copy link
Contributor

jhoeller commented Apr 4, 2019

This is by design: see #19901, applying to unique constructors where no resolution of overloaded constructors is necessary. Constructors are generally different from fields and methods there in that we cannot simply skip them if they are not required, we can only fall back to a less greedy constructor or fail. And for modern design purposes, a single unique constructor or factory method with a collection argument is common enough to warrant specific support. Before, it was rather surprising that a collection (typically among several arguments) cannot be empty in a scenario where there is no fallback.

Point taken though that this is not properly reflected in the documentation. I'll turn this into a doc ticket.

@jhoeller jhoeller self-assigned this Apr 4, 2019
@jhoeller jhoeller added type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 4, 2019
@jhoeller jhoeller added this to the 5.1.7 milestone Apr 4, 2019
@therob
Copy link
Author

therob commented Apr 4, 2019

Thanks for the quick reply. I didn't see this change.

The injection of an empty collection would be pretty much always an error in our application. Can you recommend a way to implement a safety net for this case?

At the moment i can see two possible ways to do this:

  1. Use an own ConstructorResolver. This would result in much duplicated code as it is not visible to the application.
  2. Implement a BeanPostProcessor which does this check via reflection.

Both seem to me more like a workaround (or rather a hack). It would be nice to have this behavior configurable. AbstractAutowireCapableBeanFactory could probably be a good spot for such a flag.

@jhoeller
Copy link
Contributor

jhoeller commented Apr 4, 2019

Why not simply put a custom assertion into your constructors where the incoming collections must not be empty? That way each such constructor may decide upon its expected state. I see that as generally recommendable since those constructors may also be called from outside of the framework, e.g. programmatically built, and should validate their incoming configuration state in any case.

@therob
Copy link
Author

therob commented Apr 4, 2019

The only points i have against this solution is that it can be forgotten and this validation is done by the framework for other dependencies. I use the injection of collections to get different strategies which get dynamically selected at runtime. Currently we have no use case in our application where it would make sense to have no strategy.

@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Apr 9, 2019
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: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

3 participants