Skip to content

Commit

Permalink
Ignore duplicate @⁠Priority values when determining highest priority
Browse files Browse the repository at this point in the history
Prior to this commit, DefaultListableBeanFactory's
determineHighestPriorityCandidate() method sometimes failed to
determine the highest priority candidate if duplicate priority
candidates were detected whose priority was not the highest priority in
the candidate set. In addition, the bean registration order affected
the outcome of the algorithm: if the highest priority was detected
before other duplicate priorities were detected, the algorithm
succeeded in determining the highest priority candidate.

This commit addresses those shortcomings by ignoring duplicate
@⁠Priority values unless the duplication is for the highest priority
encountered, in which case a NoUniqueBeanDefinitionException is still
thrown to signal that multiple beans were found with the same "highest
priority".

Closes gh-33733
  • Loading branch information
sbrannen committed Oct 19, 2024
1 parent 67d78eb commit d72c8b3
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1776,12 +1776,15 @@ else if (candidateLocal) {
* @param requiredType the target dependency type to match against
* @return the name of the candidate with the highest priority,
* or {@code null} if none found
* @throws NoUniqueBeanDefinitionException if multiple beans are detected with
* the same highest priority value
* @see #getPriority(Object)
*/
@Nullable
protected String determineHighestPriorityCandidate(Map<String, Object> candidates, Class<?> requiredType) {
String highestPriorityBeanName = null;
Integer highestPriority = null;
boolean highestPriorityConflictDetected = false;
for (Map.Entry<String, Object> entry : candidates.entrySet()) {
String candidateBeanName = entry.getKey();
Object beanInstance = entry.getValue();
Expand All @@ -1790,13 +1793,12 @@ protected String determineHighestPriorityCandidate(Map<String, Object> candidate
if (candidatePriority != null) {
if (highestPriority != null) {
if (candidatePriority.equals(highestPriority)) {
throw new NoUniqueBeanDefinitionException(requiredType, candidates.size(),
"Multiple beans found with the same priority ('" + highestPriority +
"') among candidates: " + candidates.keySet());
highestPriorityConflictDetected = true;
}
else if (candidatePriority < highestPriority) {
highestPriorityBeanName = candidateBeanName;
highestPriority = candidatePriority;
highestPriorityConflictDetected = false;
}
}
else {
Expand All @@ -1806,6 +1808,13 @@ else if (candidatePriority < highestPriority) {
}
}
}

if (highestPriorityConflictDetected) {
throw new NoUniqueBeanDefinitionException(requiredType, candidates.size(),
"Multiple beans found with the same highest priority (" + highestPriority +
") among candidates: " + candidates.keySet());

}
return highestPriorityBeanName;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1729,18 +1729,73 @@ void mapInjectionWithPriority() {
assertThat(bean.getBeanName()).isEqualTo("bd1");
}

/**
* {@code determineHighestPriorityCandidate()} should reject duplicate
* priorities for the highest priority detected.
*
* @see #getBeanByTypeWithMultipleNonHighestPriorityCandidates()
*/
@Test
void getBeanByTypeWithMultiplePriority() {
void getBeanByTypeWithMultipleHighestPriorityCandidates() {
lbf.setDependencyComparator(AnnotationAwareOrderComparator.INSTANCE);
RootBeanDefinition bd1 = new RootBeanDefinition(HighPriorityTestBean.class);
RootBeanDefinition bd2 = new RootBeanDefinition(HighPriorityTestBean.class);
RootBeanDefinition bd2 = new RootBeanDefinition(LowPriorityTestBean.class);
RootBeanDefinition bd3 = new RootBeanDefinition(HighPriorityTestBean.class);
lbf.registerBeanDefinition("bd1", bd1);
lbf.registerBeanDefinition("bd2", bd2);
lbf.registerBeanDefinition("bd3", bd3);

assertThatExceptionOfType(NoUniqueBeanDefinitionException.class)
.isThrownBy(() -> lbf.getBean(TestBean.class))
.withMessageContaining("Multiple beans found with the same priority")
.withMessageContaining("5"); // conflicting priority
.withMessageContaining("Multiple beans found with the same highest priority (5) among candidates: ");
}

/**
* {@code determineHighestPriorityCandidate()} should ignore duplicate
* priorities for any priority other than the highest, and the order in
* which beans is declared should not affect the outcome.
*
* @see #getBeanByTypeWithMultipleHighestPriorityCandidates()
*/
@Test // gh-33733
void getBeanByTypeWithMultipleNonHighestPriorityCandidates() {
getBeanByTypeWithMultipleNonHighestPriorityCandidates(
PriorityService1.class,
PriorityService2A.class,
PriorityService2B.class,
PriorityService3.class
);

getBeanByTypeWithMultipleNonHighestPriorityCandidates(
PriorityService3.class,
PriorityService2B.class,
PriorityService2A.class,
PriorityService1.class
);

getBeanByTypeWithMultipleNonHighestPriorityCandidates(
PriorityService2A.class,
PriorityService1.class,
PriorityService2B.class,
PriorityService3.class
);

getBeanByTypeWithMultipleNonHighestPriorityCandidates(
PriorityService2A.class,
PriorityService3.class,
PriorityService1.class,
PriorityService2B.class
);
}

private void getBeanByTypeWithMultipleNonHighestPriorityCandidates(Class<?>... classes) {
lbf.setDependencyComparator(AnnotationAwareOrderComparator.INSTANCE);
for (Class<?> clazz : classes) {
lbf.registerBeanDefinition(clazz.getSimpleName(), new RootBeanDefinition(clazz));
}

PriorityService bean = lbf.getBean(PriorityService.class);
assertThat(bean).isExactlyInstanceOf(PriorityService1.class);
}

@Test
Expand Down Expand Up @@ -3500,6 +3555,26 @@ public KnowsIfInstantiated() {
}


interface PriorityService {
}

@Priority(1)
static class PriorityService1 implements PriorityService {
}

@Priority(2)
static class PriorityService2A implements PriorityService {
}

@Priority(2)
static class PriorityService2B implements PriorityService {
}

@Priority(3)
static class PriorityService3 implements PriorityService {
}


@Priority(5)
private static class HighPriorityTestBean extends TestBean {
}
Expand Down

0 comments on commit d72c8b3

Please sign in to comment.