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: https://github.com/spring-projects/spring-boot/pull/11063

Issue Links: - #19538 ImportAware.setImportMetadata not invoked if import inherited from superclass with negative condition - #21690 TrackedConditionEvaluator skips loading bean definitions for configuration classes that should not be skipped

Referenced from: commits https://github.com/spring-projects/spring-framework/commit/47383fce97e8748481e7ffe0896b08366d058ddb, https://github.com/spring-projects/spring-framework/commit/08c95fbcb38645486309074eaef7fcf42b846e38

1 votes, 5 watchers

Comment From: 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.

Comment From: 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.

Comment From: 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.

Comment From: jhoeller

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