Skip to content

Accept empty Collection<Component> injection for single constructor scenarios [SPR-15338] #19901

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

Closed
spring-projects-issues opened this issue Mar 9, 2017 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 9, 2017

Caleb Cushing opened SPR-15338 and commented

So I was going to do this

	private Collection<JChannel> jChannels;

	public HealthCheckService( Collection<JChannel> jChannels )
	{
	     this.jChannels = jChannels;	
	}

with the production profile this should have a bean, but in development it does not have any. I was expecting Spring to hand me an empty collection, but instead it handed me an exception.

	private Collection<JChannel> jChannels = Collections.emptySet();

	public HealthCheckService(
		final ServletContext context,
		final OrganizationDao organizationDao )
	{
		this.context = Objects.requireNonNull( context );
		this.organizationDao = Objects.requireNonNull( organizationDao );
	}

        @Autowired( required = false )
	void setjChannels( final Collection<JChannel> jChannels ) {
		this.jChannels = jChannels;
	}

I'm processing it as such, or maybe a stream later, we'll see.

private Optional<State> jgroupsAreOnline() {
     for ( JChannel jChannel : jChannels )
     {
          String state = jChannel.getState();
     }
     return Optional.empty();
}

Maybe this behavior could change in Spring 5, I could see if you annotated the collection with @Required to still require a bean, but to me in general, a collection should not be null, and generally should not be required to have elements. Your opinion may differ.


Affects: 4.3.7

Issue Links:

Referenced from: commits spring-projects/spring-boot@c98bb40

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented May 3, 2017

Juergen Hoeller commented

This turns out to be quite involved since collections may consist of beans as elements or be beans themselves (typically beans of type Map but also shared Set or List setups)... Passing in an empty collection only really makes sense in the former case but we can't easily differentiate the declaration intent. Additionally, we have the overloading problem for constructors and factory methods where the algorithm doesn't consider a signature satisfiable if an argument is null, which is also applicable to collection arguments.

I'm afraid we'll have to leave this as-is for 5.0, just allowing for individual optionality markers for particular collection arguments (e.g. @Autowired(required=false) at the parameter level). We'll revisit this along with #16046 for 5.1 then.

@spring-projects-issues
Copy link
Collaborator Author

Caleb Cushing commented

IIRC one of my issues, that I don't seem to have documented was actually because I had to use setter injection instead of constructor injection, not sure what I tried there though.

thought, perhaps this could be solved by some sort of "DefaultForMissingBeanCollection" (maybe an argument annotation?, or just @Autowired( missing=EMPTY) ), also re-reading, not entirely certain I would want to deal with those scenarios differently, also, how would you know, if they're missing which it's supposed to be.

I've no need for a solution in 5.0 specifically, thanks for looking into it .

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 23, 2018

Juergen Hoeller commented

This is effectively superseded by #16046 now, in favor of iterable/stream-style access via ObjectProvider declarations.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 23, 2018

Juergen Hoeller commented

As mentioned on #16046, a fallback to an empty (but mutable) List / Set / Map in case of a single-constructor scenario (in particular when not annotated at all) seems to work fine and does not break existing tests. Effectively this makes a case that previously threw a misconfiguration-assuming bean creation exception work, so shouldn't cause regressions. It is also in line with #16046's original intent for factory methods, so goes nicely with that as well.

The limitations are that this only works for constructor and factory method arguments in the unique-constructor / unique-factory-method case where we would otherwise be unable to construct an instance at all. With overloads, we still resort to backing out of resolution with null, and for fields and setters, we still don't populate them at all when not resolvable (allowing for empty collections to be set as default values on the underlying fields there).

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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants