TestRestTemplateContextCustomizer and WebTestClientContextCustomizer can cause early FactoryBean instantiation

[darkMechanicum]

Hi!

Description

A assume there is a bug in spring-boot-test, related to TestRestTemplate BeanDefinition initialization.
To be precise, class TestRestTemplateContextCustomizer#TestRestTemplateRegistrar in it's method postProcessBeanDefinitionRegistry calls BeanFactoryUtils#beanNamesForTypeIncludingAncestors.
As beanNamesForTypeIncludingAncestors says, that's method invocation can cause FactoryBean initialization (and their dependencies, of course). And, in context of BeanDefinitionRegistryPostProcessor this, I guess, is incorrect behavior.

Reproducing

One file reproducer is as follows:

@Configuration
@RunWith(SpringJUnit4ClassRunner.class)
@SpringBootTest(classes = SingleFileReproducerTest.class,
        webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
@EnableAutoConfiguration
public class SingleFileReproducerTest {

    @Test
    public void dummy() {}

    @Component
    public static class SimpleFactoryBean implements FactoryBean {
        public SimpleFactoryBean(ApplicationContext context) {}
        public Object getObject() { return new Object(); }
        public Class<?> getObjectType() { return Object.class; }
    }
}

Note, when webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT is commented out, test passes, as there is no need for TestRestTemplate.

Versions

1. org.springframework.boot.spring-boot-starter:2.1.2.RELEASE
2. org.springframework.boot.spring-boot-starter-web:2.1.2.RELEASE
3. org.springframework.boot.spring-boot-starter-test:2.1.2.RELEASE
4. junit.junit:4.12
5. Java 1.8

[wilkinsona]

Thanks for the minimal sample. We'll see what we can do, but I would recommend that you avoid injecting dependencies into a FactoryBean as it can cause some unexpected and hard to diagnose problems such as spring-projects/spring-framework#13893. I would also recommend that you provide as much type information as possible in your FactoryBean's signature and in the Class that's returned from getObjectType(). Returning Object.class and not parameterising the factory bean is causing the container to do more work to determine the type of the bean than will be produced by the factory.

[wilkinsona]

WebTestClientRegistrar from WebTestClientContextCustomizer contains similar logic and will, presumably, cause a similar problem.

[darkMechanicum]

Thanks for advice about FactoryBean. As for example - I filled it with such error prone modifications as Object.class just for it's size.

By the way, I found this behavior while using custom JpaRepositoryFactoryBean. If I understand correctly, dependencies should be injected in FactoryBean by constructor? Or there should be no autowiring at all and one should create it manually, in @Bean method, for example?

[wilkinsona]

The biggest benefit of minimising the dependencies of a factory bean are when that bean is a @Component and you are relying on the dependencies being autowired into the constructor. If you want to inject dependencies, I'd recommend using a @Bean method instead.

Interestingly, when using a @Bean method, Framework will end its attempt to determine the type created by the factory bean more aggressively which helps to avoid the problem you're seeing here. That difference in behaviour is a little surprising to me. I've opened spring-projects/spring-framework#22409 to explore that part of what's going on here.

[wilkinsona]

There's a change coming in Framework (thanks, @jhoeller) but we should review our use of BeanFactoryUtils#beanNamesForTypeIncludingAncestors and try to avoid triggering eager initialisation nonetheless.