spring-cloud / spring-cloud-netflix

Integration with Netflix OSS components
http://cloud.spring.io/spring-cloud-netflix/
Apache License 2.0
4.86k stars 2.44k forks source link

org.springframework.cloud.netflix.eureka.EurekaClientConfigBean not robust DI make start failed #3612

Closed leonzhangxf closed 5 years ago

leonzhangxf commented 5 years ago

Problem description

org.springframework.cloud.netflix.eureka.EurekaClientConfigBean the class in package dependency:

<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-netflix-eureka-client</artifactId>
<version>2.1.1.RELEASE</version>

has a property

@Autowired(required = false)
PropertyResolver propertyResolver;

used to get experimental properties:

        @Override
    public String getExperimental(String name) {
        if (this.propertyResolver != null) {
            return this.propertyResolver.getProperty(PREFIX + ".experimental." + name,
                    String.class, null);
        }
        return null;
    }

When the spring context just got one org.springframework.core.env.PropertyResolver, it just work fine. While when there are more than one org.springframework.core.env.PropertyResolver bean contained in the spring context, it would be work out a failed start spring boot application. (the senarios mention before could be replay by add two org.springframework.core.env.PropertyResolver spring bean into context. Mime replay is performed when I use spring cloud netflix and spring boot dubbo with the mavne dependency below).

        <dependency>
            <groupId>org.apache.dubbo</groupId>
            <artifactId>dubbo-spring-boot-starter</artifactId>
            <version>2.7.1</version>
        </dependency>
        <dependency>
            <groupId>org.apache.dubbo</groupId>
            <artifactId>dubbo</artifactId>
            <version>2.7.1</version>
        </dependency>

Suggestion

The aim of the org.springframework.cloud.netflix.eureka.EurekaClientConfigBean to use org.springframework.core.env.PropertyResolver is just to get the experimental property from the context property map. So it could just be replaced by a List<PropertyResolver> list bean injection rather a single bean, in case the problem I described before happend.

spencergibb commented 5 years ago

Have multiple property resolvers is unusual.

ryanjbaxter commented 5 years ago

Is dubbo-spring-boot-starter adding its own PropertyResolver?

leonzhangxf commented 5 years ago

Is dubbo-spring-boot-starter adding its own PropertyResolver?

Yes, in the first module(I use the second one in my project which involve the first one dependency):

<groupId>org.apache.dubbo</groupId>
<artifactId>dubbo-spring-boot-autoconfigure</artifactId>
<version>2.7.1</version>
<dependency>
       <groupId>org.apache.dubbo</groupId>
       <artifactId>dubbo-spring-boot-starter</artifactId>
       <version>2.7.1</version>
</dependency>

And there is a class named org.apache.dubbo.spring.boot.autoconfigure.DubboRelaxedBinding2AutoConfiguration contains a PropertyResolver bean production for spring container. The snippet shows below:

    @Bean(name = BASE_PACKAGES_PROPERTY_RESOLVER_BEAN_NAME)
    public PropertyResolver dubboScanBasePackagesPropertyResolver(ConfigurableEnvironment environment) {
        ConfigurableEnvironment propertyResolver = new AbstractEnvironment() {
            protected void customizePropertySources(MutablePropertySources propertySources) {
                Map<String, Object> dubboScanProperties = PropertySourcesUtils.getSubProperties(environment, DUBBO_SCAN_PREFIX);
                propertySources.addLast(new MapPropertySource("dubboScanProperties", dubboScanProperties));
            }
        };
        ConfigurationPropertySources.attach(propertyResolver);
        return new DelegatingPropertyResolver(propertyResolver);
    }
spencergibb commented 5 years ago

Again, this is highly unusual. Just saying inject a list isn't helpful either. Which one would we use to resolve properties?

spencergibb commented 5 years ago

I'd say dubbo probably should change. There are better strategies for adding property sources

leonzhangxf commented 5 years ago

Yes, the scenario is unsuual and can be handled by the user who use cloud eureka and dubbo. But the cloud eureka module and the dubbo spring boot module both use the properly way to provide property by the PropertyResolver interface. So I do not think there is a wrong in both modules. Anyway I resolved the problem by add one more bean configuration with Primary annotation. I will take talk at the dubbo-spring-boot-starter repo.