Harmonize build-time parsing of the bean factory

Previously, BuildTimeBeanDefinitionsRegistrar was adding configuration
class parsing manually and triggered it to build the list of bean
definitions. Only certain framework callbacks were invoked.

This commit harmonizes its processing so that it is as close as possible
to what the regular runtime context would do. As a result, additional
callbacks are invoked at build time which had a subtle impact on how
bean definition types were discovered. To alleviate with that,
BeanClassBeanDefinitionPostProcessor makes sure to resolve the bean
class early if necessary.

Closes gh-1213

Author: snicoll

spring-aot/src/main/java/org/springframework/aot/context/bootstrap/generator/DefaultBeanDefinitionSelector.java

@@ -49,6 +49,9 @@ public DefaultBeanDefinitionSelector(List<String> excludeTypes) {
 		// TODO Make a better split between the AOT and runtime parts otherwise too much reflection is required, so for now @Autowired on fields/setters and event listeners are not properly supported
 		this.excludedBeanNames.add(AnnotationConfigUtils.AUTOWIRED_ANNOTATION_PROCESSOR_BEAN_NAME);
 		this.excludedBeanNames.add(AnnotationConfigUtils.COMMON_ANNOTATION_PROCESSOR_BEAN_NAME);
+
+		// Only used during configuration class parsing
+		this.excludedBeanNames.add("org.springframework.boot.autoconfigure.internalCachingMetadataReaderFactory");
 	}
 
 	@Override

spring-aot/src/main/java/org/springframework/context/annotation/BeanClassBeanDefinitionPostProcessor.java

@@ -0,0 +1,39 @@
+package org.springframework.context.annotation;
+
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.BeanFactory;
+import org.springframework.beans.factory.BeanFactoryAware;
+import org.springframework.beans.factory.config.ConfigurableBeanFactory;
+import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.core.Ordered;
+import org.springframework.core.annotation.Order;
+
+/**
+ * Ensure that {@link RootBeanDefinition#hasBeanClass()} can be safely used by bean
+ * definition processors.
+ *
+ * @author Stephane Nicoll
+ */
+@Order(Ordered.HIGHEST_PRECEDENCE)
+class BeanClassBeanDefinitionPostProcessor implements BeanDefinitionPostProcessor, BeanFactoryAware {
+
+	private ClassLoader classLoader;
+
+	@Override
+	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
+		this.classLoader = ((ConfigurableBeanFactory) beanFactory).getBeanClassLoader();
+	}
+
+	@Override
+	public void postProcessBeanDefinition(String beanName, RootBeanDefinition beanDefinition) {
+		if (!beanDefinition.hasBeanClass()) {
+			try {
+				beanDefinition.resolveBeanClass(this.classLoader);
+			}
+			catch (ClassNotFoundException ex) {
+				// ignore
+			}
+		}
+	}
+
+}

spring-aot/src/main/java/org/springframework/context/annotation/BuildTimeBeanDefinitionsRegistrar.java

@@ -19,6 +19,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Predicate;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -29,18 +30,22 @@
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor;
 import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.context.support.ApplicationContextAccessor;
 import org.springframework.context.support.GenericApplicationContext;
 import org.springframework.core.io.support.SpringFactoriesLoader;
 import org.springframework.core.type.AnnotationMetadata;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 /**
- * Parse the {@link Configuration @Configuration classes} and provide the bean definitions
- * at build time.
+ * Process a {@link GenericApplicationContext} refresh phase up to a point where all the
+ * bean definitions have been created, but prior to actually creating bean instances.
+ * <p/>
+ * This is used at build time to get an overview of an application context.
  *
  * @author Stephane Nicoll
  * @see ConditionEvaluationStateReport
+ * @see BeanDefinitionPostProcessor
  */
 public class BuildTimeBeanDefinitionsRegistrar {
 
@@ -56,54 +61,15 @@ public ConfigurableListableBeanFactory processBeanDefinitions(GenericApplication
 		Assert.notNull(context, "Context must not be null");
 		Assert.state(!context.isActive(), () -> "Context must not be active");
 		if (logger.isDebugEnabled()) {
-			logger.debug("Parsing configuration classes");
+			logger.debug("Processing bean factory");
 		}
-		parseConfigurationClasses(context);
-		ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
-		invokeBeanDefinitionRegistryPostProcessors(beanFactory);
-		resolveBeanDefinitionTypes(beanFactory);
+		ConfigurableListableBeanFactory beanFactory = ApplicationContextAccessor.prepareContext(context);
 		postProcessBeanDefinitions(beanFactory);
+		removeBeanDefinitionRegistryPostProcessors(beanFactory);
 		registerImportOriginRegistryIfNecessary(beanFactory);
 		return beanFactory;
 	}
 
-	private void parseConfigurationClasses(GenericApplicationContext context) {
-		ConfigurationClassPostProcessor configurationClassPostProcessor = new ConfigurationClassPostProcessor();
-		configurationClassPostProcessor.setApplicationStartup(context.getApplicationStartup());
-		configurationClassPostProcessor.setBeanClassLoader(context.getClassLoader());
-		configurationClassPostProcessor.setEnvironment(context.getEnvironment());
-		configurationClassPostProcessor.setResourceLoader(context);
-		configurationClassPostProcessor.postProcessBeanFactory(context.getBeanFactory());
-	}
-
-	private void invokeBeanDefinitionRegistryPostProcessors(ConfigurableListableBeanFactory beanFactory) {
-		BeanDefinitionRegistry registry = (BeanDefinitionRegistry) beanFactory;
-		Map<String, BeanDefinitionRegistryPostProcessor> candidates = beanFactory.getBeansOfType(BeanDefinitionRegistryPostProcessor.class, true, false);
-		candidates.forEach((beanName, postProcessor) -> {
-			postProcessor.postProcessBeanDefinitionRegistry(registry);
-			if (beanFactory.containsBeanDefinition(beanName)) {
-				BeanDefinition beanDefinition = beanFactory.getBeanDefinition(beanName);
-				if (beanDefinition.getRole() == BeanDefinition.ROLE_INFRASTRUCTURE) {
-					((BeanDefinitionRegistry) beanFactory).removeBeanDefinition(beanName);
-					logger.debug("Removed " + BeanDefinitionRegistryPostProcessor.class.getSimpleName() + " with bean name " + beanName);
-				}
-				else {
-					logger.warn(BeanDefinitionRegistryPostProcessor.class.getSimpleName() + " with bean name "
-							+ beanName + " is going to be invoked again at runtime, set a role infrastructure to avoid this");
-				}
-			}
-		});
-	}
-
-	private void resolveBeanDefinitionTypes(ConfigurableListableBeanFactory beanFactory) {
-		if (logger.isDebugEnabled()) {
-			logger.debug("Resolving types for " + beanFactory.getBeanDefinitionCount() + " bean definitions");
-		}
-		for (String name : beanFactory.getBeanDefinitionNames()) {
-			beanFactory.getType(name);
-		}
-	}
-
 	private void postProcessBeanDefinitions(ConfigurableListableBeanFactory beanFactory) {
 		if (logger.isDebugEnabled()) {
 			logger.debug("Post processing " + beanFactory.getBeanDefinitionCount() + " bean definitions");
@@ -118,6 +84,29 @@ private void postProcessBeanDefinitions(ConfigurableListableBeanFactory beanFact
 		}
 	}
 
+	private void removeBeanDefinitionRegistryPostProcessors(ConfigurableListableBeanFactory beanFactory) {
+		Map<String, BeanDefinitionRegistryPostProcessor> candidates = beanFactory.getBeansOfType(BeanDefinitionRegistryPostProcessor.class, true, false);
+		candidates.forEach((beanName, postProcessor) -> {
+			if (!removeBeanDefinition(beanFactory, beanName, (bd) -> bd.getRole() == BeanDefinition.ROLE_INFRASTRUCTURE)) {
+				logger.warn(BeanDefinitionRegistryPostProcessor.class.getSimpleName() + " with bean name "
+						+ beanName + " is going to be invoked again at runtime, set a role infrastructure to avoid this");
+			}
+		});
+	}
+
+	private boolean removeBeanDefinition(ConfigurableListableBeanFactory beanFactory, String beanName,
+			Predicate<BeanDefinition> condition) {
+		if (beanFactory.containsBeanDefinition(beanName)) {
+			BeanDefinition beanDefinition = beanFactory.getMergedBeanDefinition(beanName);
+			if (condition.test(beanDefinition)) {
+				((BeanDefinitionRegistry) beanFactory).removeBeanDefinition(beanName);
+				logger.debug("Removed bean definition with name " + beanName);
+				return true;
+			}
+		}
+		return false;
+	}
+
 	private void registerImportOriginRegistryIfNecessary(ConfigurableListableBeanFactory beanFactory) {
 		if (logger.isDebugEnabled()) {
 			logger.debug("Retrieving import origins if necessary");

spring-aot/src/main/java/org/springframework/context/support/ApplicationContextAccessor.java

@@ -0,0 +1,28 @@
+package org.springframework.context.support;
+
+import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+
+/**
+ * Accessor to privileged methods of {@link GenericApplicationContext}.
+ *
+ * @author Stephane Nicoll
+ */
+public class ApplicationContextAccessor {
+
+	/**
+	 * Prepare the specified {@link GenericApplicationContext} up to a point where it is
+	 * ready to create bean instances
+	 * @param context the context to prepare
+	 * @return the processed bean factory
+	 */
+	public static ConfigurableListableBeanFactory prepareContext(GenericApplicationContext context) {
+		context.prepareRefresh();
+		ConfigurableListableBeanFactory beanFactory = context.obtainFreshBeanFactory();
+		context.prepareBeanFactory(beanFactory);
+		context.postProcessBeanFactory(beanFactory);
+		context.invokeBeanFactoryPostProcessors(beanFactory);
+		context.registerBeanPostProcessors(beanFactory);
+		return beanFactory;
+	}
+
+}

spring-aot/src/main/resources/META-INF/spring.factories

@@ -1,4 +1,5 @@
 org.springframework.context.annotation.BeanDefinitionPostProcessor=\
+org.springframework.context.annotation.BeanClassBeanDefinitionPostProcessor,\
 org.springframework.data.RepositoryFactoryBeanPostProcessor,\
 org.springframework.plugin.PluginRegistryFactoryBeanPostProcessor,\
 org.springframework.context.annotation.CommonAnnotationBeanDefinitionPostProcessor

spring-aot/src/test/java/org/springframework/aot/context/bootstrap/generator/event/EventListenerMethodRegistrationGeneratorTests.java

@@ -103,7 +103,7 @@ void writeEventListenersRegistrationWithEventListeners() {
 
 	@Test
 	void writeEventListenersRegistrationWithPackageProtectedEventListener() {
-		GenericApplicationContext context = new GenericApplicationContext(prepareBeanFactory());
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBeanDefinition("configuration", BeanDefinitionBuilder.rootBeanDefinition(ProtectedEventListenerConfiguration.class)
 				.getBeanDefinition());
 		BuildTimeBeanDefinitionsRegistrar registrar = new BuildTimeBeanDefinitionsRegistrar();

spring-aot/src/test/java/org/springframework/aot/context/bootstrap/generator/infrastructure/BootstrapInfrastructureWriterTests.java

@@ -29,6 +29,7 @@
 import org.springframework.aot.context.bootstrap.generator.sample.callback.NestedImportConfiguration;
 import org.springframework.aot.context.bootstrap.generator.test.CodeSnippet;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.BuildTimeBeanDefinitionsRegistrar;
 import org.springframework.context.annotation.ContextAnnotationAutowireCandidateResolver;
 import org.springframework.context.annotation.samples.simple.ConfigurationTwo;
@@ -47,39 +48,39 @@ class BootstrapInfrastructureWriterTests {
 
 	@Test
 	void writeInfrastructureSetAutowireCandidateResolver() {
-		assertThat(writeInfrastructure(new GenericApplicationContext(), createBootstrapContext()))
+		assertThat(writeInfrastructure(createBootstrapContext()))
 				.contains("context.getDefaultListableBeanFactory().setAutowireCandidateResolver(new ContextAnnotationAutowireCandidateResolver());")
 				.hasImport(ContextAnnotationAutowireCandidateResolver.class);
 	}
 
 	@Test
 	void writeInfrastructureWithNoImportAwareCandidateDoesNotRegisterBean() {
-		assertThat(writeInfrastructure(new GenericApplicationContext(), createBootstrapContext()))
+		assertThat(writeInfrastructure(createBootstrapContext()))
 				.doesNotContain(ImportAwareBeanPostProcessor.class.getSimpleName());
 	}
 
 	@Test
 	void writeInfrastructureWithNoImportAwareMappingDoesNotRegisterBean() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean(ConfigurationTwo.class);
-		assertThat(writeInfrastructure(context, createBootstrapContext()))
+		assertThat(writeInfrastructure(createBootstrapContext(), context))
 				.doesNotContain(ImportAwareBeanPostProcessor.class.getSimpleName());
 	}
 
 	@Test
 	void writeInfrastructureWithImportAwareRegisterBean() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean(ImportConfiguration.class);
-		assertThat(writeInfrastructure(context, createBootstrapContext()))
+		assertThat(writeInfrastructure(createBootstrapContext(), context))
 				.contains("context.getBeanFactory().addBeanPostProcessor(createImportAwareBeanPostProcessor());");
 	}
 
 	@Test
 	void writeInfrastructureWithImportAwareRegisterCreateMethod() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean(ImportConfiguration.class);
 		BootstrapWriterContext bootstrapContext = createBootstrapContext();
-		writeInfrastructure(context, bootstrapContext);
+		writeInfrastructure(bootstrapContext, context);
 		assertThat(generateCode(bootstrapContext.getBootstrapClass("com.example")).lines()).contains(
 				"  private ImportAwareBeanPostProcessor createImportAwareBeanPostProcessor() {",
 				"    Map<String, String> mappings = new LinkedHashMap<>();",
@@ -92,11 +93,11 @@ void writeInfrastructureWithImportAwareRegisterCreateMethod() {
 
 	@Test
 	void writeInfrastructureWithSeveralImportAwareInstances() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean(ImportConfiguration.class);
 		context.registerBean(AsyncConfiguration.class);
 		BootstrapWriterContext bootstrapContext = createBootstrapContext();
-		writeInfrastructure(context, bootstrapContext);
+		writeInfrastructure(bootstrapContext, context);
 		assertThat(generateCode(bootstrapContext.getBootstrapClass("com.example")).lines()).contains(
 				"  private ImportAwareBeanPostProcessor createImportAwareBeanPostProcessor() {",
 				"    Map<String, String> mappings = new LinkedHashMap<>();",
@@ -111,10 +112,10 @@ void writeInfrastructureWithSeveralImportAwareInstances() {
 
 	@Test
 	void writeInfrastructureWithNestedClass() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean(NestedImportConfiguration.class);
 		BootstrapWriterContext bootstrapContext = createBootstrapContext();
-		writeInfrastructure(context, bootstrapContext);
+		writeInfrastructure(bootstrapContext, context);
 		assertThat(generateCode(bootstrapContext.getBootstrapClass("com.example")).lines()).contains(
 				"  private ImportAwareBeanPostProcessor createImportAwareBeanPostProcessor() {",
 				"    Map<String, String> mappings = new LinkedHashMap<>();",
@@ -127,17 +128,17 @@ void writeInfrastructureWithNestedClass() {
 
 	@Test
 	void writeInfrastructureWithNoLifecycleMethodsDoesNotRegisterBean() {
-		assertThat(writeInfrastructure(new GenericApplicationContext(), createBootstrapContext()))
+		assertThat(writeInfrastructure(createBootstrapContext()))
 				.doesNotContain(InitDestroyBeanPostProcessor.class.getSimpleName())
 				.doesNotContain("createInitDestroyBeanPostProcessor");
 	}
 
 	@Test
 	void writeInfrastructureWithLifecycleMethodsRegisterCreateMethod() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean("testBean", InitDestroySampleBean.class);
 		BootstrapWriterContext bootstrapContext = createBootstrapContext();
-		writeInfrastructure(context, bootstrapContext);
+		writeInfrastructure(bootstrapContext, context);
 		assertThat(generateCode(bootstrapContext.getBootstrapClass("com.example")).lines()).contains(
 				"  private InitDestroyBeanPostProcessor createInitDestroyBeanPostProcessor(",
 				"      ConfigurableBeanFactory beanFactory) {",
@@ -151,13 +152,17 @@ void writeInfrastructureWithLifecycleMethodsRegisterCreateMethod() {
 
 	@Test
 	void writeInfrastructureWithLifecycleMethodsRegisterBean() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean("testBean", InitDestroySampleBean.class);
-		assertThat(writeInfrastructure(context, createBootstrapContext())).contains(
+		assertThat(writeInfrastructure(createBootstrapContext(), context)).contains(
 				"context.getBeanFactory().addBeanPostProcessor(createInitDestroyBeanPostProcessor(context.getBeanFactory()));");
 	}
 
-	private CodeSnippet writeInfrastructure(GenericApplicationContext context, BootstrapWriterContext writerContext) {
+	private CodeSnippet writeInfrastructure(BootstrapWriterContext writerContext) {
+		return writeInfrastructure(writerContext, new AnnotationConfigApplicationContext());
+	}
+
+	private CodeSnippet writeInfrastructure(BootstrapWriterContext writerContext, GenericApplicationContext context) {
 		ConfigurableListableBeanFactory beanFactory = this.registrar.processBeanDefinitions(context);
 		BootstrapInfrastructureWriter writer = new BootstrapInfrastructureWriter(beanFactory, writerContext);
 		return CodeSnippet.of(writer::writeInfrastructure);

spring-aot/src/test/java/org/springframework/aot/context/bootstrap/generator/test/ApplicationContextAotProcessorTester.java

@@ -27,6 +27,7 @@
 
 import org.springframework.aot.context.bootstrap.generator.ApplicationContextAotProcessor;
 import org.springframework.aot.context.bootstrap.generator.infrastructure.DefaultBootstrapWriterContext;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.support.GenericApplicationContext;
 import org.springframework.util.ClassUtils;
 
@@ -53,7 +54,7 @@ public ApplicationContextAotProcessorTester(Path directory) {
 	}
 
 	public ContextBootstrapStructure process(Class<?>... candidates) {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		for (Class<?> candidate : candidates) {
 			context.registerBean(generateShortName(candidate), candidate);
 		}

spring-aot/src/test/java/org/springframework/aot/context/origin/BeanFactoryStructureAnalyzerTests.java

@@ -18,6 +18,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.BuildTimeBeanDefinitionsRegistrar;
 import org.springframework.context.annotation.samples.simple.ConfigurationOne;
 import org.springframework.context.support.GenericApplicationContext;
@@ -33,13 +34,13 @@ class BeanFactoryStructureAnalyzerTests {
 
 	@Test
 	void analyzeSimpleStructure() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean(ConfigurationOne.class);
 		BeanFactoryStructure structure = analyze(context);
 		assertThat(structure).isNotNull();
-		assertThat(structure.getDescriptors()).containsOnlyKeys("beanOne", ConfigurationOne.class.getName());
+		assertThat(structure.getDescriptors()).containsKeys("beanOne", "configurationOne");
 		assertThat(structure.getDescriptors().get("beanOne").getOrigins())
-				.containsOnly(ConfigurationOne.class.getName());
+				.containsOnly("configurationOne");
 	}
 
 	private BeanFactoryStructure analyze(GenericApplicationContext context) {

spring-aot/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationPackagesBeanDefinitionOriginAnalyzerTests.java

@@ -20,6 +20,7 @@
 
 import org.springframework.aot.context.origin.BeanDefinitionDescriptor.Type;
 import org.springframework.aot.context.origin.BeanFactoryStructureAnalysis;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.BuildTimeBeanDefinitionsRegistrar;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.support.GenericApplicationContext;
@@ -37,7 +38,7 @@ class AutoConfigurationPackagesBeanDefinitionOriginAnalyzerTests {
 
 	@Test
 	void analyseAutoConfigurePackages() {
-		GenericApplicationContext context = new GenericApplicationContext();
+		GenericApplicationContext context = new AnnotationConfigApplicationContext();
 		context.registerBean(SampleConfiguration.class);
 		new BuildTimeBeanDefinitionsRegistrar().processBeanDefinitions(context);
 		BeanFactoryStructureAnalysis analysis = BeanFactoryStructureAnalysis.of(context.getBeanFactory());