spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.55k stars 40.55k forks source link

ApplicationContextRunner has inconsistent behaviour with duplicate auto-configuration class names #17963

Open michael-simons opened 5 years ago

michael-simons commented 5 years ago

Given the following configurations:

package com.example.demo.a;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

import com.example.outside_app.SomeService;

@Configuration
public class SomeConfig {

    @Bean
    public SomeService someServiceFromA() {
        return new SomeService();
    }

}

and in another package, under the same, simple name:

package com.example.demo.b;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

import com.example.outside_app.SomeService;

@Configuration
public class SomeConfig {

    @Bean
    public SomeService someServiceFromB() {
        return new SomeService();
    }

}

Spring Framework refuses to work with two configuration classes having the same bean name (someConfig). This is fine, the error is usable.

However, the ApplicationContextRunnerdoes not do this and takes in the following configuration

@Test
public void f2() {
    new ApplicationContextRunner()
        .withUserConfiguration(com.example.demo.a.SomeConfig.class, com.example.demo.b.SomeConfig.class)
    .run(ctx -> {
        assertThat(ctx).hasBean("someServiceFromB");
        assertThat(ctx).hasBean("someServiceFromA");
    });
}

And fails the test by silently ignoring or overwriting the first configuration given.

While it would have been noticed in this case, you don't notice it when you test for the absence of one bean and the presence of another. As one config has maybe silently dropped, conditions are not tested.

This happens also with overlapping autoconfigurations having the same class name and this is where I actually found the issue:

Spring Boot and or Spring Framework do apparently support autoconfigurations with the same simple class name.

Given this bean

package com.example.outside_app;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class JpaRepositoriesAutoConfiguration {

    @Bean
    public SomeService someServiceFromAutoConfig() {
        return new SomeService();
    }

}

and Spring Factories containing

org.springframework.boot.autoconfigure.EnableAutoConfiguration = com.example.outside_app.JpaRepositoriesAutoConfiguration

I can test my application:

@RunWith(SpringRunner.class)
@SpringBootTest
public class DemoApplicationTests {

    @Autowired
    private Map<String, SomeService> someServices;

    @Test
    public void contextLoads() {
        assertThat(someServices).containsKeys("someServiceFromAutoConfig");
    }

}

However, the ApplicationContextRunner disagrees and

@Test
    public void onlyAutoConfig() {

        new ApplicationContextRunner()
            .withConfiguration(
                AutoConfigurations.of(
                    org.springframework.boot.autoconfigure.data.jpa.JpaRepositoriesAutoConfiguration.class,
                    com.example.outside_app.JpaRepositoriesAutoConfiguration.class
                ))
            .run(ctx -> assertThat(ctx).hasBean("someServiceFromAutoConfig"));
    }

fails.

While the duplicate names need good reasons and are of course a thing to debate, the application context runner should agree during tests with the framework to prevent errors in custom starters or applications, depending on what is actually tested.

I have attached the demo project.

autoconfigissue.zip

philwebb commented 5 years ago

I wonder if this might also be related to SpringApplication.allowBeanDefinitionOverriding which we added in 2.1.

michael-simons commented 5 years ago

At least I ran into the problem of not being allowed to overwrite beans because my Test with faulty. Error was in my config, but I didn’t spot it. Happens easily with the behavior of the test runner.

snicoll commented 5 years ago

The problem is that Configurations (and its AutoConfigurations implementation) do not register classes the same way AutoConfigurationImportSelector does. The former calls a simple register on the context (with a sensible order) while the latter is an ImportSelector that returns the name of the classes to load.

The net result is that the framework does something different. The register is at the same level as a bean so the simple name of the class is used (just like any other @Component really), while the import selector uses the FQN.

snicoll commented 5 years ago

I've tried a few things and I am not sure how to tackle this problem. The context is created manually with a list of configuration classes that I know at runtime so I can't really use an ImportSelector here.

@jhoeller do you have an idea for us? I've tried to use a BeanFactoryPostProcessor but it doesn't process a custom bean definition as a Configuration class, probably because ConfigurationClassPostProcessor has already ran.

snicoll commented 5 years ago

I've been looking at fixing the issue (see https://github.com/snicoll/spring-boot/commit/035cdda1a7f509d43178d3f57910b08fea5695f5). The idea is to mimic what ImportSelector (and DeferredImportSelector) does. It turns out that having a FQN as a bean name is a fallback behaviour of the core container when the bean name is null.

There is a timing issue that's a bit odd but I might have nailed it. When the auto-configuration backs off because a condition with REGISTER_PHASE doesn't match, it looks like the configuration class is processed anyway and blows up.

You can reproduce by running SpringDataWebAutoConfigurationTests#autoConfigurationBacksOffInNonWebApplicationContexts

With the change it fails with

org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'org.springframework.boot.autoconfigure.data.web.SpringDataWebAutoConfiguration': Unsatisfied dependency expressed through constructor parameter 0; nested exception is org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'org.springframework.boot.autoconfigure.data.web.SpringDataWebProperties' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {}
    at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:787)
    at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:226)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1359)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1205)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:557)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:517)
    at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:323)

Everything works as expected as far as I can see if we replace the BeanDefinitionRegistryPostProcessor by regular calls to a GenericApplicationContext (see commented code in the branch). Unfortunately, we can't use that situation as a runner with a traditional deployment has a context that doesn't inherit from that hierarchy.