Autowiring fails if multiple non-highest `@Priority` beans exist with same priority

[xinbimingjingtai]

Overview

When I was reading the source code for DefaultListableBeanFactory#determineHighestPriorityCandidate(), I found that when multiple beans have the same priority but not the highest priority, the highest priority bean cannot be obtained correctly.

Example

Consider the following scenario:

public interface IFacade {
}

@Priority(10)
@Service
public class Facade0 implements IFacade{
}

@Priority(9)
@Service
public class Facade1 implements IFacade{
}

@Priority(9)
@Service
public class Facade2 implements IFacade{
}

@Priority(8)
@Service
public class Facade3 implements IFacade{
}

@Component
public class Config {

    @Autowired
    private IFacade facade;

}

Proposal

I know that @Qualifier or @Primary can be used to handle this situation, but in this case, the highest priority bean is only Facade3, which should be correctly injected into Config instead of throwing an exception.

Can you consider adding the highest priority results to a List (refer to the code below) and throwing an exception if there are multiple beans with the highest priority?

    @Nullable
    protected String determineHighestPriorityCandidate(Map<String, Object> candidates, Class<?> requiredType) {
        List<String> highestPriorityBeanNames = new ArrayList<>();
        Integer highestPriority = null;
        for (Map.Entry<String, Object> entry : candidates.entrySet()) {
            String candidateBeanName = entry.getKey();
            Object beanInstance = entry.getValue();
            if (beanInstance != null) {
                Integer candidatePriority = getPriority(beanInstance);
                if (candidatePriority != null) {
                    if (!highestPriorityBeanNames.isEmpty()) {
                        if (candidatePriority.equals(highestPriority)) {
                            highestPriorityBeanNames.add(candidateBeanName);
                        }
                        else if (candidatePriority < highestPriority) {
                            highestPriorityBeanNames.clear();
                            highestPriorityBeanNames.add(candidateBeanName);
                            highestPriority = candidatePriority;
                        }
                    }
                    else {
                        highestPriorityBeanNames.add(candidateBeanName);
                        highestPriority = candidatePriority;
                    }
                }
            }
        }
        if (highestPriorityBeanNames.size() > 1) {
            throw new NoUniqueBeanDefinitionException(requiredType, candidates.size(),
                    "Multiple beans found with the same priority ('" + highestPriority +
                            "') among candidates: " + candidates.keySet());
        }
        return highestPriorityBeanNames.isEmpty() ? null : highestPriorityBeanNames.get(0);
    }

[sbrannen]

Hi @xinbimingjingtai,

Congratulations on submitting your first issue for the Spring Framework! 👍

I have reduced your example to the following standalone test class.

@SpringJUnitConfig
class PriorityBeanTests {

    @Test
    void test(@Autowired Service service) {
        assertThat(service).isExactlyInstanceOf(Service1.class);
    }

    interface Service {
    }

    @Priority(1)
    static class Service1 implements Service {
    }

    @Priority(2)
    static class Service2A implements Service {
    }

    @Priority(2)
    static class Service2B implements Service {
    }

    @Priority(3)
    static class Service3 implements Service {
    }

    @Configuration
    @Import({
        Service3.class,
        Service2B.class,
        Service2A.class,
        Service1.class,
    })
    static class Config {
    }
}

As you hinted at, that fails with the following exception.

Caused by: org.springframework.beans.factory.NoUniqueBeanDefinitionException: No qualifying bean of type 'example.PriorityBeanTests$Service' available: Multiple beans found with the same priority ('2') among candidates: [example.PriorityBeanTests$Service3, example.PriorityBeanTests$Service2B, example.PriorityBeanTests$Service2A, example.PriorityBeanTests$Service1]
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.determineHighestPriorityCandidate(DefaultListableBeanFactory.java:2013)
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.determineAutowireCandidate(DefaultListableBeanFactory.java:1928)
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1599)
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1514)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.resolveDependency(AbstractAutowireCapableBeanFactory.java:472)

However, if I change the order in which the Service beans are registered so that the highest priority is detected before any duplicates are detected, then the test passes.

The following are two such configurations that pass.

@Import({
    Service1.class,
    Service2A.class,
    Service2B.class,
    Service3.class,
})

@Import({
    Service2A.class,
    Service3.class,
    Service1.class,
    Service2B.class,
})

I would consider that a bug in the implementation of determineHighestPriorityCandidate(): the bean registration order should not affect the outcome.

However, one could consider it a user error if any beans of the same type are assigned the same priority via @Priority. So, I'll discuss with @jhoeller to decide if we want to change the semantics for that.

[xinbimingjingtai]

Yes, I agree with you, but I still think there may exist special cases where different beans have the same priority.

[sbrannen]

This has been fixed in 6.1.x (for inclusion in 6.1.15) in commit d72c8b32b75eeb19ef51baa8a51f35caaa80044f.

Can you consider adding the highest priority results to a List (refer to the code below) and throwing an exception if there are multiple beans with the highest priority?

It turns out a simple boolean flag was all that was needed to implement the check. See the aforementioned commit for details.

[xinbimingjingtai]

Yes, your solution is clearer and more simple.