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

Direct autowiring of existing collection subtype bean if ApplicationContext contains beans of type T #30022

Closed
yvasyliev opened this issue Feb 24, 2023 · 8 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@yvasyliev
Copy link

yvasyliev commented Feb 24, 2023

The current focus of this issue is described in #30022 (comment).

For background information, the original issue title and description follow.


Previous title: "Cannot autowire java.util.Queue<T> if ApplicationContext contains beans of type T"

Affects: spring-context 5.3.25


I'm not able to autowire java.util.Queue in console app using spring-context.

There is a quick code example:

public interface Action {
    void doAction();
}
public class Action1 implements Action {
    @Override
    public void doAction() {
        System.out.println(this.getClass().getName());
    }
}
public class Action2 implements Action {
    @Override
    public void doAction() {
        System.out.println(this.getClass().getName());
    }
}
public class ActionManager {
    @Autowired
    private Queue<Action> actionQueue;

    public void doActions() {
        actionQueue.forEach(Action::doAction);
    }
}
@Configuration
public class Config {
    @Bean
    public ActionManager actionManager() {
        return new ActionManager();
    }

    @Bean
    public Queue<Action> actionQueue() {
        Queue<Action> actionQueue = new ArrayDeque<>();
        actionQueue.add(action1());
        actionQueue.add(action2());
        return actionQueue;
    }

    @Bean
    public Action action1() {
        return new Action1();
    }

    @Bean
    public Action action2() {
        return new Action2();
    }
}
public class Main {
    public static void main(String[] args) {
        ApplicationContext context = new AnnotationConfigApplicationContext(Config.class);
        ActionManager actionManager = context.getBean(ActionManager.class);
        actionManager.doActions();
    }
}

When I run this code, I receive the next error in console:

Feb 24, 2023 9:16:11 AM org.springframework.context.support.AbstractApplicationContext refresh
WARNING: Exception encountered during context initialization - cancelling refresh attempt: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'actionManager': Unsatisfied dependency expressed through field 'actionQueue'; nested exception is org.springframework.beans.ConversionNotSupportedException: Failed to convert value of type 'java.util.LinkedHashMap$LinkedValues' to required type 'java.util.Queue'; nested exception is java.lang.IllegalStateException: Cannot convert value of type 'java.util.LinkedHashMap$LinkedValues' to required type 'java.util.Queue': no matching editors or conversion strategy found
Exception in thread "main" org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'actionManager': Unsatisfied dependency expressed through field 'actionQueue'; nested exception is org.springframework.beans.ConversionNotSupportedException: Failed to convert value of type 'java.util.LinkedHashMap$LinkedValues' to required type 'java.util.Queue'; nested exception is java.lang.IllegalStateException: Cannot convert value of type 'java.util.LinkedHashMap$LinkedValues' to required type 'java.util.Queue': no matching editors or conversion strategy found
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.resolveFieldValue(AutowiredAnnotationBeanPostProcessor.java:660)
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(AutowiredAnnotationBeanPostProcessor.java:640)
	at org.springframework.beans.factory.annotation.InjectionMetadata.inject(InjectionMetadata.java:119)
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.postProcessProperties(AutowiredAnnotationBeanPostProcessor.java:399)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1431)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:619)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:542)
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:335)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:333)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:955)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:918)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:583)
	at org.springframework.context.annotation.AnnotationConfigApplicationContext.<init>(AnnotationConfigApplicationContext.java:93)
	at app.Main.main(Main.java:8)
Caused by: org.springframework.beans.ConversionNotSupportedException: Failed to convert value of type 'java.util.LinkedHashMap$LinkedValues' to required type 'java.util.Queue'; nested exception is java.lang.IllegalStateException: Cannot convert value of type 'java.util.LinkedHashMap$LinkedValues' to required type 'java.util.Queue': no matching editors or conversion strategy found
	at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:76)
	at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:45)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveMultipleBeans(DefaultListableBeanFactory.java:1471)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1349)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1311)
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.resolveFieldValue(AutowiredAnnotationBeanPostProcessor.java:657)
	... 15 more
Caused by: java.lang.IllegalStateException: Cannot convert value of type 'java.util.LinkedHashMap$LinkedValues' to required type 'java.util.Queue': no matching editors or conversion strategy found
	at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:262)
	at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:73)
	... 20 more

Note 1: If I replace @Autowired with @javax.annotation.Resource, the code will work.
Note 2: If I replace java.util.Queue with java.util.Set, the code will work.

Since the issue is reproduced for java.util.Queue only, it seems like a bug.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 24, 2023
@sbrannen sbrannen changed the title Failed to autowire java.util.Queue Cannot autowire java.util.Queue<T> if ApplicationContext contains beans of type T Feb 24, 2023
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 24, 2023
@sbrannen sbrannen self-assigned this Feb 24, 2023
@sbrannen
Copy link
Member

sbrannen commented Feb 24, 2023

This is a known limitation and effectively a duplicate of #16794. See the discussions in that issue for details.

Known workarounds:

  1. As you mentioned, you can use @Resource instead of @Autowired.
  2. Alternatively, you can combine @Autowired with @Qualifier("actionQueue") in ActionManager.

Aside from those workarounds, there is also the option to remove the Action @Bean methods from your configuration, and your code will then work as expected.

The reason is that Spring will first look for beans of type Action and attempt to create a Queue collection instance from all Action beans; however, as you pointed out that results in an exception since Spring does not know how to convert from java.util.LinkedHashMap$LinkedValues to java.util.Queue.

Thus, when you remove the Action @Bean methods, Spring no longer finds any beans of type Action and instead looks for a bean of type Queue<Action> which it finds and autowires into your ActionManager.

In summary, if you do not need to have the Action instances managed as beans within the ApplicationContext, I would recommend you simply remove them. On the other hand, if you need them to be managed beans, I'd suggest you go with @Resource or the @Autowired/@Qualifier combination.

@sbrannen
Copy link
Member

To help you experiment with the options, I've reduced your examples to the following single test class.

The test passes as-is, but if you reinstate the @Bean lines the test will fail as in your original example.

@SpringJUnitConfig
class AutowiredQueueTests {

	@Autowired
	// @Autowired @Qualifier("queue")
	// @Resource
	Queue<String> queue;

	@Test
	void test() {
		assertThat(this.queue).containsExactly("foo", "bar");
	}

	@Configuration
	static class Config {

		// @Bean
		String foo() {
			return "foo";
		}

		// @Bean
		String bar() {
			return "bar";
		}

		@Bean
		Queue<String> queue() {
			Queue<String> queue = new ArrayDeque<>();
			queue.add(foo());
			queue.add(bar());
			return queue;
		}

	}

}

@sbrannen
Copy link
Member

sbrannen commented Feb 24, 2023

As a proof of concept, I inserted the following lines of code...

// If a single bean of the exact collection type exists, do not resolve beans of the elementType into a
// new collection. Rather, fall back to autowiring the single bean of the exact collection type as-is.
if (determineAutowireCandidate(findAutowireCandidates(beanName, type, descriptor), descriptor) != null) {
	return null;
}

... into DefaultListableBeanFactory#resolveMultipleBeans(DependencyDescriptor, String, Set<String>, TypeConverter) here:

Map<String, Object> matchingBeans = findAutowireCandidates(beanName, elementType,

That appears to achieve the desired effect and does not cause any other tests in Spring Framework's test suite to fail -- though it is a change in behavior and can cause existing applications to fail or behave differently.

@jhoeller, putting aside the comments you made in #16794 against changing this behavior, what are your thoughts on supporting such use cases in a future version of Spring Framework?

@sbrannen sbrannen added the type: enhancement A general enhancement label Feb 24, 2023
@sbrannen
Copy link
Member

As a side note for @yvasyliev and anyone else reading this issue...

If you want Spring to collect beans of a certain type into a collection type that is not supported by Spring out of the box, you can always register your own custom Converter.

For example, if we want Spring to find all beans of type String and collect them into a Queue (with concrete type ArrayDeque), we could modify the previous test case as follows.

@SpringJUnitConfig
class AutowiredQueueTests {

	@Autowired
	Queue<String> queue;

	@Test
	void test() {
		assertThat(this.queue).containsExactly("foo", "bar");
	}

	@Configuration
	static class Config {

		@Bean
		String foo() {
			return "foo";
		}

		@Bean
		String bar() {
			return "bar";
		}

		@Bean
		ConversionService conversionService() {
			ConversionServiceFactoryBean factory = new ConversionServiceFactoryBean();
			factory.setConverters(Set.of(new CollectionToQueueConverter()));
			factory.afterPropertiesSet();
			return factory.getObject();
		}

	}

	@SuppressWarnings("rawtypes")
	static class CollectionToQueueConverter implements Converter<Collection<?>, Queue<?>> {

		@Override
		public Queue<?> convert(Collection<?> source) {
			return new ArrayDeque<>(source);
		}
	}

}

Note, however, that the above CollectionToQueueConverter is very basic (always creates an ArrayDeque) and completely ignores generics. In addition, there is no longer a @Bean Queue<String> queue() method since Spring is now creating the Queue.

@sbrannen
Copy link
Member

sbrannen commented Feb 27, 2023

In light of this issue, #29987, as well as the popularity of issues such as #16794 (and all of the related issues linked from that issue), we have decided to revisit this topic for Spring Framework 6.1.

Specifically, our goal is to support direct injection of a collection bean even if the bean factory contains beans of the type contained in the collection bean.

Things to consider:

  • investigate if it makes sense to introduce a simple check as demonstrated in above.
  • investigate if it makes sense to support (or require?) @Primary on the collection bean to signal that the existing collection bean should be injected instead of a dynamically created collection consisting of beans of that type in the bean factory.
  • investigate if the same kind of support can be provided for map beans.

@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 27, 2023
@sbrannen sbrannen added this to the 6.1.x milestone Feb 27, 2023
@sbrannen sbrannen changed the title Cannot autowire java.util.Queue<T> if ApplicationContext contains beans of type T Support autowiring of existing Collection<T> bean if ApplicationContext contains beans of type T Feb 27, 2023
@sbrannen sbrannen removed their assignment Feb 27, 2023
@jhoeller jhoeller self-assigned this Aug 8, 2023
@jhoeller jhoeller modified the milestones: 6.1.x, 6.1.0-M4 Aug 8, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Aug 9, 2023

It's worth noting that you may also mark the individual Action beans as @Bean(autowireCandidate=false). This keeps them around as managed beans and for direct references as in the actionQueue() implementation but does not consider them for type-based injection through @Autowired anywhere.

@jhoeller
Copy link
Contributor

jhoeller commented Aug 9, 2023

I'm inclined to repurpose this ticket for narrowing our collection of beans algorithm in 6.1, along the lines of #19684: just supporting Collection, Set and List declarations for multiple bean retrieval but not attempting resolution of any other collection types for that purpose. We never defined this to work to begin with, it was rather accidental that collection sub-interfaces other than Set and List were considered as well. This means that for all other collection types, in particular for ones that we cannot resolve anyway, we directly attempt matching actual beans of a collection type to begin with.

For the time being and for enforcing direct matching even for a Collection, Set and List injection point, there are the above-mentioned options of specific qualifiers and/or autowireCandidate=false markers.

@jhoeller jhoeller changed the title Support autowiring of existing Collection<T> bean if ApplicationContext contains beans of type T Direct autowiring of existing collection subtype bean if ApplicationContext contains beans of type T Aug 9, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Aug 12, 2023

This is implemented as a three-step check now: first searching for multiple beans in case of well-known collection types (which is performance-relevant since this is the most common use of collections at injection points), then for direct matches with collection beans (for standard as well as custom collection types), and only then as a fallback also custom collection declarations for multiple beans. So we effectively attempt to resolve e.g. a Queue or SortedSet directly now, and only as a fallback consider it for multiple beans, where as a Collection/Set/List declaration is considered for multiple beans first.

In practice, please do not mix individual beans and collection beans of the same type unless you use qualifiers for them. If there are just individual beans or just a collection bean for a certain dependency type, the resolution is always straightforward.

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

4 participants