Reject conditional @ComponentScan declarations that rely on the REGISTER_BEAN phase

[wangxing-git]

spring boot version

2.1.5.RELEASE

TestConfig.java

package org.xyattic.boot.calkin.demo.oauth2.server.config;

import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.core.io.Resource;

/**
 * @author wangxing
 * @create 2019/6/28
 */
@Configuration
@Import(TestConfig.Test.class)
@ConditionalOnBean(Resource.class)
public class TestConfig {

    @ConditionalOnClass(Resource.class)
    @ComponentScan("org.xyattic.boot.calkin.demo.oauth2.test")
    class Test {

    }

}

The package org.xyattic.boot.calkin.demo.oauth2.test by default is not scanned.
When I activated it under certain conditions, I found that @ComponentScan was also activated when the condition was not met.

TestConfig:
      Did not match:
         - @ConditionalOnBean (types: org.springframework.core.io.Resource; SearchStrategy: all) did not find any beans of type org.springframework.core.io.Resource (OnBeanCondition)

   TestConfig.Test:
      Did not match:
         - Ancestor org.xyattic.boot.calkin.demo.oauth2.server.config.TestConfig did not match (ConditionEvaluationReport.AncestorsMatchedCondition)
      Matched:
         - @ConditionalOnClass found required class 'org.springframework.core.io.Resource' (OnClassCondition)

The log shows as not matching.
And the following will not accidentally activate.

@Configuration
@Import(TestConfig.Test.class)
//@ConditionalOnBean(Resource.class)
@ConditionalOnProperty("test.enabled")
public class TestConfig {

    @ConditionalOnClass(Resource.class)
    @ComponentScan("org.xyattic.boot.calkin.demo.oauth2.test")
    class Test {

    }

}

[wilkinsona]

Thanks for the report. The behaviour that you have observed is part of Spring Framework.

In the first example the unexpected behaviour is occurring because Framework does not consistently consider the conditions on TestConfig when evaluating the conditions on its Test inner-class. When it's deciding whether or not to process the @ComponentScan it only considers the conditions on Test. Later on, when it's deciding whether to process any @Bean definitions on Test it does consider the conditions on TestConfig. In implementation terms this is because of the use of TrackedConditionEvaluator in the latter case.

The second example avoids the problem of the first example because @ConditionalOnProperty is a different sort of condition to @ConditionalOnBean. Condition evaluation is split into two phases, configuration parsing and bean registration. @ConditionalOnProperty can be evaluated in the configuration parsing phase, whereas @ConditionalOnBean can only be evaluated in the bean registration phase. In the second example, when TestConfig is processed during configuration parsing, @ConditionalOnProperty does not match so TestConfig is skipped along with its inner-classes. By contrast, in the first example @ConditionalOnBean is not considered during the configuration parsing phase so Test and its @ComponentScan are then processed.

In short, we need to move this to spring-projects/spring-framework so that the Framework team can take a look.

[echooymxq]

version:5.2.1.RELEASE

@ConditionalOnBean(EnableSvcConfiguration.Marker.class)
@ComponentScan("com.github.bzi.service")
@Configuration
public class SvcAutoConfiguration {
}

In my case, when i use ConditionalOnBean meet condition, the @ComponentScan can't be activated.

[wangxing-git]

version:5.2.1.RELEASE

@ConditionalOnBean(EnableSvcConfiguration.Marker.class)
@ComponentScan("com.github.bzi.service")
@Configuration
public class SvcAutoConfiguration {
}

In my case, when i use ConditionalOnBean meet condition, the @ComponentScan can't be activated.

@echooymxq
My example is by the outer class @Import inner class, @ComponentScan is marked on the inner class, and the condition of the outer class uses @ConditionalOnBean and the condition is not met, but @ComponentScan on the inner class is still activated.

[snicoll]

Juergen and I brainstormed based on the same above and we've decided to reject the scenario altogether, that is when we detect that @CompoentScan is tied with a condition that's in the REGISTER_BEAN phase.

Consider the following example:

@AutoConfiguration
@ConditionalOnBean(String.class)
@ComponentScan("com.acme")
public class TestAutoConfiguration { ... }

When a String bean is not available:

   TestAutoConfiguration:
      Did not match:
         - @ConditionalOnBean (types: java.lang.String; SearchStrategy: all) did not find any beans of type java.lang.String (OnBeanCondition)

When a String bean is available:

TestAutoConfiguration:
      Did not match:
         - @ConditionalOnBean (types: java.lang.String; SearchStrategy: all) did not find any beans of type java.lang.String (OnBeanCondition)
      Matched:
         - @ConditionalOnBean (types: java.lang.String; SearchStrategy: all) found bean 'testString' (OnBeanCondition)

In both cases, component scan does not happen. We can see that the condition is evaluated twice there, one at the PARSE_CONFIGURATION phase, and once again at the REGISTER_BEAN phase. This is due to the component scan that forces the condition to be evaluated.

If the scan is moved to a nested config class that has an outer class with a REGISTER_BEAN condition:

@AutoConfiguration
@ConditionalOnBean(String.class)
public class TestAutoConfiguration {

	@Configuration
	@ComponentScan("com.acme")
	static class NestedConfiguration {
	}
}

Then it keeps scanning as reported here.

In general, conditional component scan is not a great idea as it has to happen early in the lifecycle of the bean factory setup, so that its effect can apply to other conditions. Mixing a late condition, such as ConditionalOnBean with an early processing can't be supported and we'll fail hard when we detect such use case.

[snicoll]

The change made in #27077 is "harmonizing" things a little bit. In both cases now the scan is always applied as it runs in the PARSE_CONFIGURATION phase and does no longer force REGISTER_BEAN conditions to be evaluated. We should probably replace the check there by something that collects the condition and throw a dedicated exception if a REGISTER_BEAN phase condition is found. The remark on the wrong conditions being detected in that related issue is to be taken into account to not introduce yet another problem.