Configuration class's superclass may be incorrectly skipped if first importer isn't skipped till register bean phase of condition evaluation [SPR-16217]

[spring-projects-issues]

Andy Wilkinson opened SPR-16217 and commented

ConfigurationClassParser populates its knownSuperclasses map during the configuration phase of condition evaluation. This means that a subclass that has a register bean phase condition can cause the superclass to be skipped even if another superclass that's encountered later would have included it.

The above is perhaps best illustrated by some tests:

import org.junit.Test;

import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ConditionContext;
import org.springframework.context.annotation.Conditional;
import org.springframework.context.annotation.ConfigurationCondition;
import org.springframework.context.annotation.Import;
import org.springframework.core.type.AnnotatedTypeMetadata;

public class KnownSuperclassesBug {

	@Test
	public void baseConfigurationIsIncludedWhenFirstSuperclassReferenceIsSkippedInRegisterBeanPhase() {
		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
				RegisterBeanPhaseImportingConfiguration.class)) {
			context.getBean("someBean");
		}
	}

	@Test
	public void baseConfigurationIsIncludedWhenFirstSuperclassReferenceIsSkippedInParseConfigurationPhase() {
		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
				ParseConfigurationPhaseImportingConfiguration.class)) {
			context.getBean("someBean");
		}
	}

	public static class RegisterBeanPhaseCondition implements ConfigurationCondition {

		@Override
		public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
			return false;
		}

		@Override
		public ConfigurationPhase getConfigurationPhase() {
			return ConfigurationPhase.REGISTER_BEAN;
		}

	}

	public static class ParseConfigurationPhaseCondition
			implements ConfigurationCondition {

		@Override
		public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
			return false;
		}

		@Override
		public ConfigurationPhase getConfigurationPhase() {
			return ConfigurationPhase.PARSE_CONFIGURATION;
		}

	}

	@Import({ RegisterBeanPhaseConditionConfiguration.class, BarConfiguration.class })
	public static class RegisterBeanPhaseImportingConfiguration {

	}

	@Import({ ParseConfigurationPhaseConditionConfiguration.class,
			BarConfiguration.class })
	public static class ParseConfigurationPhaseImportingConfiguration {

	}

	public static class BaseConfiguration {

		@Bean
		public String someBean() {
			return "foo";
		}

	}

	@Conditional(RegisterBeanPhaseCondition.class)
	public static class RegisterBeanPhaseConditionConfiguration
			extends BaseConfiguration {

	}

	@Conditional(ParseConfigurationPhaseCondition.class)
	public static class ParseConfigurationPhaseConditionConfiguration
			extends BaseConfiguration {

	}

	public static class BarConfiguration extends BaseConfiguration {

	}

}

My expectation is that both tests will pass. As things stand baseConfigurationIsIncludedWhenFirstSuperclassReferenceIsSkippedInRegisterBeanPhase fails and baseConfigurationIsIncludedWhenFirstSuperclassReferenceIsSkippedInParseConfigurationPhase() passes.

The first test fails because RegisterBeanPhaseConditionConfiguration is processed and BaseConfiguration is added to the knownSuperclasses map. When BarConfiguration is processed BaseConfiguration is already in the knownSuperclasses map so its "import" via BarConfiguration is lost. When the register bean phase condition on RegisterBeanPhaseConditionConfiguration is evaluated it doesn't match so it's skipped along with BaseConfiguration that it imports.

---

Affects: 4.3.12, 5.0.1

Reference URL: spring-projects/spring-boot#11063

Issue Links:

• ImportAware.setImportMetadata not invoked if import inherited from superclass with negative condition [SPR-14972] #19538 ImportAware.setImportMetadata not invoked if import inherited from superclass with negative condition
• TrackedConditionEvaluator skips loading bean definitions for configuration classes that should not be skipped [SPR-17153] #21690 TrackedConditionEvaluator skips loading bean definitions for configuration classes that should not be skipped

Referenced from: commits 47383fc, 08c95fb

1 votes, 5 watchers

[spring-projects-issues]

Andy Wilkinson commented

I've lowered the priority to minor. I marked it as major as I thought it was blocking the merge of the referenced Spring Boot pull request. I've just figured out that's not the case.

[spring-projects-issues]

Juergen Hoeller commented

This turns out to be surprisingly non-trivial: The knownSuperclasses handling in ConfigurationClassParser has a deep assumption about locally managing included/excluded superclasses. In order for that to be overridable in the REGISTER_BEAN phase, we'd have to export some common state to ConfigurationClassBeanDefinitionReader... or get rid of the special superclass handling to begin with (but that would change the existing semantics, letting later occurences of the superclass override earlier bean definitions and point to the later configuration class instance).

I'll revisit this for 5.0.3.

[jhoeller]

There is a lot of complexity in the condition evaluation for configuration classes, and I'm inclined to accept the present quirks instead of making it even more complex. In practice, I'd rather recommend against scenarios with such complex assumptions to begin with.

[jhoeller]

The above said, I am going to look at the specifically narrowed #28676 which effectively supersedes this issue.