Availability of a bean definition's resolved factory method depends upon a side-effect of getBeanNamesForType which is lost when metadata caching is disabled

[wilkinsona]

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:

factoryMethodToIntrospect is set on the ConfigurationClassBeanDefinition due to a call that's made to beanFactory.getBeanNamesForType(BeanDefinitionRegistryPostProcessor.class, true, false) when invoking bean factory post-processors. With caching enabled, this merged bean definition is held in the bean factory's mergedBeanDefinitions map. It's then available when making a subsequent call to getResolvedFactoryMethod(). When caching is disabled, the merged bean definition on which factoryMethodToIntrospect is set is not held in the mergedBeanDefinitions map so the information is lost. When a new merged bean definition is subsequently retrieved via a call to getMergedBeanDefinition, there's no logic to set factoryMethodToIntrospect so it's null.

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 of getBeanNamesForType(). 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 when getResolvedFactoryMethod() is null, 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.

[jhoeller]

Why is metadata caching disabled in such a scenario? To some degree the factory method is a piece of cached metadata here...

[wilkinsona]

As far as I could tell it was being done automatically by the Spring plugin for the HotSwap Agent that's built-in to the DCEVM that was being used.