Skip to content
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

Doc: @Bean methods on @Configuration class returned from another @Bean method do not work [SPR-14602] #19171

Closed
spring-issuemaster opened this issue Aug 18, 2016 · 4 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Aug 18, 2016

Steven Schlansker opened SPR-14602 and commented

I have an @Configuration class which holds some internal configuration data. It wants to instantiate a collaborating @Configuration class, hand off some data to it, and expects its collaborator to exist. However, it seems that an @Bean method that returns a @Configuration class fails because its BeanDefinition#getBeanClassName is null, so ConfigurationClassUtils#checkConfigurationClassCandidate returns false.

@RunWith(SpringRunner.class)
@ContextConfiguration(classes=BeanDefinitionAutowireTest.Config.class)
public class BeanDefinitionAutowireTest {

    @Configuration
    public static class InnerConfig {
        @Bean
        URI someUri() {
            return URI.create("/");
        }
    }

    @Configuration
    public static class Config {
        @Bean
        public InnerConfig innerConfig() {
            return new InnerConfig();
        }
    }

    @Autowired
    URI someUri;

    @Test
    public void testUri() {
        assertNotNull(someUri);
    }
}

I feel that this should work. If for some reason it can not, it should throw an exception or otherwise fail obviously, rather than silently discarding the definitions you expect.

(Note that this test case is a little simplified, in the real case I pass constructor arguments to InnerConfig, so making things static is not an option)


Affects: 4.3.1

Issue Links:

  • #15258 Allow BeanDefinitionRegistryPostProcessor to register other BeanDefinitionRegistryPostProcessors
  • #16056 @Configuration imported via ImportBeanDefinitionRegistrar is not processed as configuration
  • #16345 @Configuration imported via @ImportResource is not processed

Referenced from: commits 9b221f5, 6fe7e56

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 19, 2016

Stéphane Nicoll commented

I am confused: why are you creating InnerConfig as both a Configuration and a Bean. Creating InnerConfig as a Bean does not make any sense since you're effectively changing the semantic of the object.

A Configuration object can be injected like any other component in spring land. I've updated your test as follows and it passes.

@RunWith(SpringRunner.class)
@ContextConfiguration(classes=BeanDefinitionAutowireTest.Config.class)
public class BeanDefinitionAutowireTest {

	@Configuration
	public static class InnerConfig {
		@Bean
		URI someUri() {
			return URI.create("/");
		}
	}

	@Configuration
	public static class AnotherConfig {
		private final InnerConfig innerConfig;

		@Autowired
		public AnotherConfig(InnerConfig innerConfig) {
			this.innerConfig = innerConfig;
			System.out.println("URI --> " + innerConfig.someUri());
		}
	}

	@Configuration
	@Import({InnerConfig.class, AnotherConfig.class})
	public static class Config {

	}

	@Autowired
	URI someUri;

	@Test
	public void testUri() {
		assertNotNull(someUri);
	}
}

Regardless of this, the behaviour of the container in such a (wrong) scenario can be improved, what do you think Juergen Hoeller?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 22, 2016

Steven Schlansker commented

Hi, thanks a lot for looking at this -- the bit that I am missing is, how do I pass constructor arguments to the InnerConfig? If you add it to the @Import you cannot. I am trying to create a parameterized configuration that may be included multiple times with differing specializations. That's how I ended up going down the (apparently unsupported) @Bean path, since then I can construct my instance.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 24, 2016

Juergen Hoeller commented

We'll have to push this back for consideration in 5.0 M3 since the impact on the underlying infrastructure is non-trivial: For example, we can't dynamically create an enhanced subclass for such a configuration class (since the factory method instantiates it), and we generally have to introspect factory method return signatures to find out whether they're configuration candidates.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 23, 2017

Juergen Hoeller commented

I'm afraid we can't reliably detect such unsupported @Bean methods on classes returned from another @Bean method since we'd have to load the return type early or even introspect the class once the bean is actually created... for every single @Bean method which almost always returns a regular bean where there is nothing to find anyway. Such a non-trivial type loading and lifecycle impact is not worth it if all we're trying to detect are unsupported scenarios. Also, we're not detecting unsupported annotations in many other cases at runtime either.

Actually supporting such configuration classes returned from factory methods on other configuration classes incurs even more trouble, including the aforementioned lack of runtime enhancement of classes. The configuration class model is really designed for the framework to instantiate such classes, not for the instances to be provided through factory methods.

Tools or IDEs may flag such invalid use of annotations, but from the core framework perspective, I'm turning this into a documentation task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.