[servicecomb-java-chassis] 01/02: [SCB-1297] NPE would be arised if you haven't given any value for servicecomb.config.ignoreResolveFailure

[ya...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

yaohaishi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git

commit 7e23a668ceb7e02cf1e3558a17e80edd9f688357
Author: l00393086 <li...@163.com>
AuthorDate: Mon Aug 12 16:06:12 2019 +0800

    [SCB-1297] NPE would be arised if you haven't given any value for servicecomb.config.ignoreResolveFailure
---
 .../config/ConfigurationSpringInitializer.java     | 17 ++++++++++-------
 .../config/TestConfigurationSpringInitializer.java | 22 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigurationSpringInitializer.java b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigurationSpringInitializer.java
index ae6d217..c7472c7 100644
--- a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigurationSpringInitializer.java
+++ b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigurationSpringInitializer.java
@@ -76,6 +76,9 @@ public class ConfigurationSpringInitializer extends PropertyPlaceholderConfigure
    */
   @Override
   public void setEnvironment(Environment environment) {
+
+    ConfigUtil.installDynamicConfig();
+
     String environmentName = generateNameForEnvironment(environment);
     LOGGER.info("Environment received, will get configurations from [{}].", environmentName);
 
@@ -83,8 +86,6 @@ public class ConfigurationSpringInitializer extends PropertyPlaceholderConfigure
 
     ConfigUtil.addExtraConfig(EXTRA_CONFIG_SOURCE_PREFIX + environmentName, extraConfig);
 
-    ConfigUtil.installDynamicConfig();
-
     setUpSpringPropertySource(environment);
   }
 
@@ -200,10 +201,10 @@ public class ConfigurationSpringInitializer extends PropertyPlaceholderConfigure
         try {
           configFromSpringBoot.put(propertyName, environment.getProperty(propertyName, Object.class));
         } catch (Exception e) {
-          if (!getIfIgnoreEnvironment()) {
-            throw new RuntimeException("set up spring property source failed.", e);
-          } else {
+          if (ignoreResolveFailure()) {
             LOGGER.warn("set up spring property source failed.", e);
+          } else {
+            throw new RuntimeException("set up spring property source failed.If you still want to start up the application and ignore the errors, you can set servicecomb.config.ignoreResolveFailure to true.", e);
           }
         }
       }
@@ -213,7 +214,9 @@ public class ConfigurationSpringInitializer extends PropertyPlaceholderConfigure
     LOGGER.debug("a none EnumerablePropertySource is ignored, propertySourceName = [{}]", propertySource.getName());
   }
 
-  private boolean getIfIgnoreEnvironment() {
-    return (Boolean) ConfigUtil.createLocalConfig().getProperty("servicecomb.config.ignoreResolveFailure");
+  private boolean ignoreResolveFailure() {
+    return DynamicPropertyFactory.getInstance()
+            .getBooleanProperty("servicecomb.config.ignoreResolveFailure", false)
+            .get();
   }
 }
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/TestConfigurationSpringInitializer.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/TestConfigurationSpringInitializer.java
index c50e01e..ac20e23 100644
--- a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/TestConfigurationSpringInitializer.java
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/TestConfigurationSpringInitializer.java
@@ -43,6 +43,9 @@ import org.springframework.core.env.CompositePropertySource;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.MapPropertySource;
 import org.springframework.core.env.MutablePropertySources;
+import org.springframework.core.env.PropertySource;
+import org.springframework.core.env.StandardEnvironment;
+import org.springframework.core.env.SystemEnvironmentPropertySource;
 import org.springframework.jndi.JndiPropertySource;
 
 import com.netflix.config.ConfigurationManager;
@@ -224,8 +227,27 @@ public class TestConfigurationSpringInitializer {
     assertEquals("value2", extraProperties.get("key2"));
   }
 
+  @Test(expected = RuntimeException.class)
+  public void shoud_throw_exception_when_given_ignoreResolveFailure_false() {
+    StandardEnvironment environment = newStandardEnvironment();
+
+    ConfigurationSpringInitializer configurationSpringInitializer = new ConfigurationSpringInitializer();
+    configurationSpringInitializer.setEnvironment(environment);
+  }
+
   private Map<String, Map<String, Object>> getExtraConfigMapFromConfigUtil() {
     return Deencapsulation
         .getField(ConfigUtil.class, "EXTRA_CONFIG_MAP");
   }
+
+  private StandardEnvironment newStandardEnvironment() {
+    Map<String, Object> envProperties = new HashMap<>();
+    envProperties.put("IFS-X", "${IFS-X}");
+    PropertySource<Map<String, Object>> systemEnvironmentPropertySource = new SystemEnvironmentPropertySource("system-env", envProperties);
+
+    StandardEnvironment environment = new StandardEnvironment();
+    environment.getPropertySources()
+            .addAfter(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, systemEnvironmentPropertySource);
+    return environment;
+  }
 }