Description
Affects: 5.1.x and 5.2.x
This is a follow-on from spring-projects/spring-boot#18440. The difference in Framework's behaviour with and without bean metadata caching is illustrated by the following tests:
package example;
import static org.assertj.core.api.Assertions.assertThat;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.context.annotation.ImportSelector;
import org.springframework.core.type.AnnotationMetadata;
class FactoryMethodResolutionTests {
@Test
void factoryMethodCanBeResolvedWithBeanMetadataCachingEnabled() {
assertThatFactoryMethodCanBeResolved(true);
}
@Test
void factoryMethodCanBeResolvedWithBeanMetadataCachingDisabled() {
assertThatFactoryMethodCanBeResolved(false);
}
private void assertThatFactoryMethodCanBeResolved(boolean cache) {
try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) {
context.getBeanFactory().setCacheBeanMetadata(cache);
context.register(ImportSelectorConfiguration.class);
context.refresh();
BeanDefinition definition = context.getBeanFactory().getMergedBeanDefinition("exampleBean");
assertThat(((RootBeanDefinition)definition).getResolvedFactoryMethod()).isNotNull();
}
}
@Configuration
@Import(ExampleImportSelector.class)
static class ImportSelectorConfiguration {
}
static class ExampleImportSelector implements ImportSelector {
@Override
public String[] selectImports(AnnotationMetadata importingClassMetadata) {
return new String[] { TestConfiguration.class.getName() };
}
}
@Configuration
static class TestConfiguration {
@Bean
@ExampleAnnotation
public ExampleBean exampleBean() {
return new ExampleBean();
}
}
static class ExampleBean {
}
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@interface ExampleAnnotation {
}
}This has only become a problem in Boot 2.2 as we're now leaning more heavily upon the resolved factory method for performance reasons.
My analysis in the Boot issue that led to me opening this issue was the following:
factoryMethodToIntrospectis set on theConfigurationClassBeanDefinitiondue to a call that's made tobeanFactory.getBeanNamesForType(BeanDefinitionRegistryPostProcessor.class, true, false)when invoking bean factory post-processors. With caching enabled, this merged bean definition is held in the bean factory'smergedBeanDefinitionsmap. It's then available when making a subsequent call togetResolvedFactoryMethod(). When caching is disabled, the merged bean definition on whichfactoryMethodToIntrospectis set is not held in themergedBeanDefinitionsmap so the information is lost. When a new merged bean definition is subsequently retrieved via a call togetMergedBeanDefinition, there's no logic to setfactoryMethodToIntrospectso it'snull.Having investigated further and written up the above, I'm now of the opinion that this is a bug in Framework.
getResolvedFactoryMethod()only works reliably due to a side-effect ofgetBeanNamesForType(). When caching is disabled, that side-effect is too short-lived to be useful. We could work around this in Boot by falling back to the old logic whengetResolvedFactoryMethod()isnull, but that would be a point-solution to what feels like a general problem.
I have added a workaround in Boot to avoid shipping a regression in 2.2, but I'd like to be able to revert that workaround in favour of a more general solution in Framework.