Skip to content

Directly registered FactoryBeans are instantiated more aggressively than those defined via @Bean methods #22409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wilkinsona opened this issue Feb 13, 2019 · 5 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

Affects: 5.1.4.RELEASE

This relates to spring-projects/spring-boot#15898.

When a FactoryBean is defined via component scanning, it is instantiated more aggressively than when it is defined via a @Bean method. The following tests should illustrate the differing behaviour:

package com.example.demo;

import org.junit.Test;
import org.springframework.beans.factory.BeanFactoryUtils;
import org.springframework.beans.factory.FactoryBean;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.Configuration;
import org.springframework.stereotype.Component;

public class AggressiveFactoryBeanInstantiationTests {

	@Test
	public void componentScannedFactoryBean() {
		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) {
			context.register(ComponentScanConfiguration.class);
			context.addBeanFactoryPostProcessor((factory) -> {
				BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory, String.class);
			});
			context.refresh();
		}
	}

	@Test
	public void beanMethodFactoryBean() {
		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) {
			context.register(BeanMethodConfiguration.class);
			context.addBeanFactoryPostProcessor((factory) -> {
				BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory, String.class);
			});
			context.refresh();
		}
	}

	@Configuration
	static class BeanMethodConfiguration {

		@Bean
		public SimpleFactoryBean simpleFactoryBean(ApplicationContext applicationContext) {
			return new SimpleFactoryBean(applicationContext);
		}

	}

	@Configuration
	@ComponentScan
	static class ComponentScanConfiguration {

	}

	@Component
	static class SimpleFactoryBean implements FactoryBean<Object> {

		public SimpleFactoryBean(ApplicationContext applicationContext) {

		}

		@Override
		public Object getObject() throws Exception {
			return new Object();
		}

		@Override
		public Class<?> getObjectType() {
			return Object.class;
		}

	}

}

beanMethodFactoryBean will successfully refresh the application context. componentScannedFactoryBean fails as an attempt is made to create SimpleFactoryBean using a non-existent default constructor:

org.springframework.beans.factory.BeanDefinitionStoreException: Failed to read candidate component class: file [/Users/awilkinson/dev/workspaces/spring-projects/spring-boot/2.1.x/gh-15898/target/test-classes/com/example/demo/AggressiveFactoryBeanInstantiationTests$BeanMethodConfiguration.class]; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'aggressiveFactoryBeanInstantiationTests.SimpleFactoryBean' defined in file [/Users/awilkinson/dev/workspaces/spring-projects/spring-boot/2.1.x/gh-15898/target/test-classes/com/example/demo/AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean.class]: Instantiation of bean failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.example.demo.AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean]: No default constructor found; nested exception is java.lang.NoSuchMethodException: com.example.demo.AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean.<init>()
	at org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider.scanCandidateComponents(ClassPathScanningCandidateComponentProvider.java:454)
	at org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider.findCandidateComponents(ClassPathScanningCandidateComponentProvider.java:316)
	at org.springframework.context.annotation.ClassPathBeanDefinitionScanner.doScan(ClassPathBeanDefinitionScanner.java:275)
	at org.springframework.context.annotation.ComponentScanAnnotationParser.parse(ComponentScanAnnotationParser.java:132)
	at org.springframework.context.annotation.ConfigurationClassParser.doProcessConfigurationClass(ConfigurationClassParser.java:287)
	at org.springframework.context.annotation.ConfigurationClassParser.processConfigurationClass(ConfigurationClassParser.java:242)
	at org.springframework.context.annotation.ConfigurationClassParser.parse(ConfigurationClassParser.java:191)
	at org.springframework.context.annotation.ConfigurationClassParser.doProcessConfigurationClass(ConfigurationClassParser.java:295)
	at org.springframework.context.annotation.ConfigurationClassParser.processConfigurationClass(ConfigurationClassParser.java:242)
	at org.springframework.context.annotation.ConfigurationClassParser.parse(ConfigurationClassParser.java:199)
	at org.springframework.context.annotation.ConfigurationClassParser.parse(ConfigurationClassParser.java:167)
	at org.springframework.context.annotation.ConfigurationClassPostProcessor.processConfigBeanDefinitions(ConfigurationClassPostProcessor.java:315)
	at org.springframework.context.annotation.ConfigurationClassPostProcessor.postProcessBeanDefinitionRegistry(ConfigurationClassPostProcessor.java:232)
	at org.springframework.context.support.PostProcessorRegistrationDelegate.invokeBeanDefinitionRegistryPostProcessors(PostProcessorRegistrationDelegate.java:275)
	at org.springframework.context.support.PostProcessorRegistrationDelegate.invokeBeanFactoryPostProcessors(PostProcessorRegistrationDelegate.java:95)
	at org.springframework.context.support.AbstractApplicationContext.invokeBeanFactoryPostProcessors(AbstractApplicationContext.java:691)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:528)
	at com.example.demo.AggressiveFactoryBeanInstantiationTests.componentScannedFactoryBean(AggressiveFactoryBeanInstantiationTests.java:22)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209)
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'aggressiveFactoryBeanInstantiationTests.SimpleFactoryBean' defined in file [/Users/awilkinson/dev/workspaces/spring-projects/spring-boot/2.1.x/gh-15898/target/test-classes/com/example/demo/AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean.class]: Instantiation of bean failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.example.demo.AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean]: No default constructor found; nested exception is java.lang.NoSuchMethodException: com.example.demo.AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean.<init>()
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.instantiateBean(AbstractAutowireCapableBeanFactory.java:1270)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1164)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck(AbstractAutowireCapableBeanFactory.java:974)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.getTypeForFactoryBean(AbstractAutowireCapableBeanFactory.java:848)
	at org.springframework.beans.factory.support.AbstractBeanFactory.isTypeMatch(AbstractBeanFactory.java:574)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doGetBeanNamesForType(DefaultListableBeanFactory.java:514)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBeanNamesForType(DefaultListableBeanFactory.java:477)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBeansOfType(DefaultListableBeanFactory.java:598)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBeansOfType(DefaultListableBeanFactory.java:590)
	at org.springframework.boot.context.TypeExcludeFilter.match(TypeExcludeFilter.java:65)
	at org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider.isCandidateComponent(ClassPathScanningCandidateComponentProvider.java:492)
	at org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider.scanCandidateComponents(ClassPathScanningCandidateComponentProvider.java:431)
	... 40 more
Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.example.demo.AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean]: No default constructor found; nested exception is java.lang.NoSuchMethodException: com.example.demo.AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean.<init>()
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:83)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.instantiateBean(AbstractAutowireCapableBeanFactory.java:1262)
	... 51 more
Caused by: java.lang.NoSuchMethodException: com.example.demo.AggressiveFactoryBeanInstantiationTests$SimpleFactoryBean.<init>()
	at java.lang.Class.getConstructor0(Class.java:3082)
	at java.lang.Class.getDeclaredConstructor(Class.java:2178)
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:78)
	... 52 more

In the @Bean method case, the attempt to determine the type produced by the FactoryBean is abandoned earlier. As far as I can tell that's due to the following logic:

// If not resolvable above and the referenced factory bean doesn't exist yet,
// exit here - we don't want to force the creation of another bean just to
// obtain a FactoryBean's object type...
if (!isBeanEligibleForMetadataCaching(factoryBeanName)) {
return null;
}

@darkMechanicum
Copy link

darkMechanicum commented Feb 14, 2019

https://github.com/spring-projects/spring-data-commons/blob/aa39b9ba3a6e64cfeff7e12fda85f69ad9ea3cd8/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionBuilder.java#L97-L101

As an additional question to this issue I want to ask about RepositoryFactoryBean behavior and instantiation process. More precise, the moment it's definition being added to the factory.
I'm curious if specifying constructor's metadata before we analyze bean is correct, and how it can affect type checking referenced in this ticket.

So, as constructor args are added manually to the definition - can such approach ruin type checking when we should invoke constructor without any dependencies injection? I mean full instantiation and bean population will be launched instead.

Just want to clarify typechecking process and it's limitations.

@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 27, 2019
@jhoeller
Copy link
Contributor

While it is uncommon - and to some degree unintended - that FactoryBean implementations have non-default constructors, we should nevertheless not make hard assumptions about it. A defensive catch block like we have for the non-singleton FactoryBean case already (since SPR-12786 / #17383 in 4.1.6) seems to do the job here, so I'll roll that into 5.1.6 (possibly backporting it to 5.0.13 and 4.3.23 as well).

@jhoeller
Copy link
Contributor

Also, it's not recommended for a BeanFactoryPostProcessor implementation to trigger unlimited type matching, i.e. a getBeanNamesForType call with the (default) allowEagerInit flag. This is what causes the particular case to fail here since our constructor-selecting AutowiredAnnotationBeanPostProcessor isn't registered at that point yet, being unable to resolve the non-default constructor case.

Nevertheless, the underlying inconsistency in our core container treatment needs to get resolved here. We just need to make sure that we're not swallowing error cases that are helpful in FactoryBean scenarios that are intended to match but fail due to incomplete configuration or the like.

@jhoeller
Copy link
Contributor

Side note: Component scanning is not really relevant here. The same effect appears when directly registering the FactoryBean implementation class a la context.register(SimpleFactoryBean.class).

@jhoeller jhoeller changed the title Factory beans defined via component scanning are instantiated more aggressively than those defined via @Bean methods Directly registered FactoryBeans are instantiated more aggressively than those defined via @Bean methods Feb 27, 2019
@jhoeller
Copy link
Contributor

I'll leave this as 5.1.6 only for the time being since our catch blocks consistently let UnsatisfiedDependencyException through now (which may have side effects) while suppressing other BeanCreationExceptions, both for the singleton and the non-singleton FactoryBean case.

mbhave added a commit to spring-projects/spring-boot that referenced this issue Mar 25, 2019
…zers

Until Spring Framework 5.1.15, a FactoryBean with a non-default constructor
defined via component scanning would cause an error. This behavior has changed
as of spring-projects/spring-framework#22409.
Regardless of this change we want to ensure that we avoid triggering eager
initialisation. `SimpleFactoryBean` has been written this way so that the tests
fail if early initialization is triggered regardless of the Spring Framework version.

Fixes gh-15898
@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants