Skip to content

Commit

Permalink
Fix annotation matching when using scoped proxies
Browse files Browse the repository at this point in the history
Update `OnBeanCondition` to check `isAutowireCandidate` on the original
bean of scoped proxy targets.

Fixes gh-43423
  • Loading branch information
philwebb committed Dec 6, 2024
1 parent 8ca8ab1 commit 86b0c76
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -202,25 +202,19 @@ private ConditionOutcome evaluateConditionalOnMissingBean(Spec<ConditionalOnMiss
}

protected final MatchResult getMatchingBeans(Spec<?> spec) {
ConfigurableListableBeanFactory beanFactory = getSearchBeanFactory(spec);
ClassLoader classLoader = spec.getContext().getClassLoader();
ConfigurableListableBeanFactory beanFactory = spec.getContext().getBeanFactory();
boolean considerHierarchy = spec.getStrategy() != SearchStrategy.CURRENT;
Set<Class<?>> parameterizedContainers = spec.getParameterizedContainers();
if (spec.getStrategy() == SearchStrategy.ANCESTORS) {
BeanFactory parent = beanFactory.getParentBeanFactory();
Assert.isInstanceOf(ConfigurableListableBeanFactory.class, parent,
"Unable to use SearchStrategy.ANCESTORS");
beanFactory = (ConfigurableListableBeanFactory) parent;
}
MatchResult result = new MatchResult();
Set<String> beansIgnoredByType = getNamesOfBeansIgnoredByType(classLoader, beanFactory, considerHierarchy,
spec.getIgnoredTypes(), parameterizedContainers);
for (String type : spec.getTypes()) {
Map<String, BeanDefinition> typeMatchedDefinitions = getBeanDefinitionsForType(classLoader,
considerHierarchy, beanFactory, type, parameterizedContainers);
Set<String> typeMatchedNames = matchedNamesFrom(typeMatchedDefinitions,
(name, definition) -> isCandidate(name, definition, beansIgnoredByType)
&& !ScopedProxyUtils.isScopedTarget(name));
(name, definition) -> !ScopedProxyUtils.isScopedTarget(name)
&& isCandidate(beanFactory, name, definition, beansIgnoredByType));
if (typeMatchedNames.isEmpty()) {
result.recordUnmatchedType(type);
}
Expand All @@ -232,7 +226,7 @@ protected final MatchResult getMatchingBeans(Spec<?> spec) {
Map<String, BeanDefinition> annotationMatchedDefinitions = getBeanDefinitionsForAnnotation(classLoader,
beanFactory, annotation, considerHierarchy);
Set<String> annotationMatchedNames = matchedNamesFrom(annotationMatchedDefinitions,
(name, definition) -> isCandidate(name, definition, beansIgnoredByType));
(name, definition) -> isCandidate(beanFactory, name, definition, beansIgnoredByType));
if (annotationMatchedNames.isEmpty()) {
result.recordUnmatchedAnnotation(annotation);
}
Expand All @@ -252,6 +246,17 @@ protected final MatchResult getMatchingBeans(Spec<?> spec) {
return result;
}

private ConfigurableListableBeanFactory getSearchBeanFactory(Spec<?> spec) {
ConfigurableListableBeanFactory beanFactory = spec.getContext().getBeanFactory();
if (spec.getStrategy() == SearchStrategy.ANCESTORS) {
BeanFactory parent = beanFactory.getParentBeanFactory();
Assert.isInstanceOf(ConfigurableListableBeanFactory.class, parent,
"Unable to use SearchStrategy.ANCESTORS");
beanFactory = (ConfigurableListableBeanFactory) parent;
}
return beanFactory;
}

private Set<String> matchedNamesFrom(Map<String, BeanDefinition> namedDefinitions,
BiPredicate<String, BeanDefinition> filter) {
Set<String> matchedNames = new LinkedHashSet<>(namedDefinitions.size());
Expand All @@ -263,9 +268,25 @@ private Set<String> matchedNamesFrom(Map<String, BeanDefinition> namedDefinition
return matchedNames;
}

private boolean isCandidate(String name, BeanDefinition definition, Set<String> ignoredBeans) {
return (!ignoredBeans.contains(name))
&& (definition == null || (definition.isAutowireCandidate() && isDefaultCandidate(definition)));
private boolean isCandidate(ConfigurableListableBeanFactory beanFactory, String name, BeanDefinition definition,
Set<String> ignoredBeans) {
return (!ignoredBeans.contains(name)) && (definition == null
|| isAutowireCandidate(beanFactory, name, definition) && isDefaultCandidate(definition));
}

private boolean isAutowireCandidate(ConfigurableListableBeanFactory beanFactory, String name,
BeanDefinition definition) {
return definition.isAutowireCandidate() || isScopeTargetAutowireCandidate(beanFactory, name);
}

private boolean isScopeTargetAutowireCandidate(ConfigurableListableBeanFactory beanFactory, String name) {
try {
return ScopedProxyUtils.isScopedTarget(name)
&& beanFactory.getBeanDefinition(ScopedProxyUtils.getOriginalBeanName(name)).isAutowireCandidate();
}
catch (NoSuchBeanDefinitionException ex) {
return false;
}
}

private boolean isDefaultCandidate(BeanDefinition definition) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
import org.springframework.context.annotation.Import;
import org.springframework.context.annotation.ImportBeanDefinitionRegistrar;
import org.springframework.context.annotation.ImportResource;
import org.springframework.context.annotation.Scope;
import org.springframework.context.annotation.ScopedProxyMode;
import org.springframework.context.support.SimpleThreadScope;
import org.springframework.core.type.AnnotationMetadata;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
Expand Down Expand Up @@ -124,6 +127,16 @@ void testAnnotationOnMissingBeanCondition() {
});
}

@Test
void testAnnotationOnMissingBeanConditionWithScopedProxy() {
this.contextRunner.withInitializer(this::registerScope)
.withUserConfiguration(ScopedExampleBeanConfiguration.class, OnAnnotationConfiguration.class)
.run((context) -> {
assertThat(context).doesNotHaveBean("bar");
assertThat(context.getBean(ScopedExampleBean.class)).hasToString("test");
});
}

@Test
void testAnnotationOnMissingBeanConditionWithEagerFactoryBean() {
// Rigorous test for SPR-11069
Expand Down Expand Up @@ -407,6 +420,10 @@ private Consumer<ConfigurableApplicationContext> exampleBeanRequirement(String..
};
}

private void registerScope(ConfigurableApplicationContext applicationContext) {
applicationContext.getBeanFactory().registerScope("test", new TestScope());
}

@Configuration(proxyBeanMethods = false)
static class OnBeanInAncestorsConfiguration {

Expand Down Expand Up @@ -665,6 +682,17 @@ String foo() {

}

@Configuration(proxyBeanMethods = false)
@TestAnnotation
static class ScopedFooConfiguration {

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

}

@Configuration(proxyBeanMethods = false)
static class NotAutowireCandidateConfig {

Expand Down Expand Up @@ -717,6 +745,12 @@ ExampleBean exampleBean() {

}

@Configuration(proxyBeanMethods = false)
@Import(ScopedExampleBean.class)
static class ScopedExampleBeanConfiguration {

}

@Configuration(proxyBeanMethods = false)
static class UnrelatedExampleBeanConfiguration {

Expand Down Expand Up @@ -893,6 +927,15 @@ public String toString() {

}

@Scope(scopeName = "test", proxyMode = ScopedProxyMode.TARGET_CLASS)
static class ScopedExampleBean extends ExampleBean {

ScopedExampleBean() {
super("test");
}

}

static class CustomExampleBean extends ExampleBean {

CustomExampleBean() {
Expand Down Expand Up @@ -931,4 +974,8 @@ public String toString() {

}

static class TestScope extends SimpleThreadScope {

}

}

0 comments on commit 86b0c76

Please sign in to comment.