You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by li...@apache.org on 2018/11/23 07:03:43 UTC

[incubator-dubbo] branch dev-metadata updated: Merge pull request #2823, Code review for ConfigurationUtils and Environment.

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

liujun pushed a commit to branch dev-metadata
in repository https://gitbox.apache.org/repos/asf/incubator-dubbo.git


The following commit(s) were added to refs/heads/dev-metadata by this push:
     new 8d8aed2  Merge pull request #2823, Code review for ConfigurationUtils and Environment.
8d8aed2 is described below

commit 8d8aed20b1f516c28d7176669a7ffbb4d8a004c3
Author: Ian Luo <ia...@gmail.com>
AuthorDate: Fri Nov 23 15:03:35 2018 +0800

    Merge pull request #2823, Code review for ConfigurationUtils and Environment.
    
    * change method signature to Configuration getConfiguration(String prefix, String id), in this way, we expose interface to the caller instead of concreate impl
    
    * remove compositeconfiguration from ConfigurationUtils
---
 .../apache/dubbo/common/config/Environment.java    |  6 ++-
 .../org/apache/dubbo/config/AbstractConfig.java    |  6 +--
 .../dubbo/config/AbstractInterfaceConfig.java      |  2 +-
 .../org/apache/dubbo/config/ServiceConfig.java     |  2 +-
 .../dubbo/configcenter/ConfigurationUtils.java     | 59 ++++------------------
 .../dubbo/container/log4j/Log4jContainer.java      | 10 ++--
 6 files changed, 25 insertions(+), 60 deletions(-)

diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java b/dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java
index a6ce939..746fbc3 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java
@@ -110,7 +110,7 @@ public class Environment {
      * @param id
      * @return
      */
-    public CompositeConfiguration getStartupCompositeConf(String prefix, String id) {
+    public Configuration getConfiguration(String prefix, String id) {
         CompositeConfiguration compositeConfiguration = new CompositeConfiguration();
         compositeConfiguration.addConfiguration(this.getSystemConfig(prefix, id));
         compositeConfiguration.addConfiguration(this.getAppExternalConfig(prefix, id));
@@ -125,6 +125,10 @@ public class Environment {
         return compositeConfiguration;
     }
 
+    public Configuration getConfiguration() {
+        return getConfiguration(null, null);
+    }
+
     private static String toKey(String prefix, String id) {
         StringBuilder sb = new StringBuilder();
         if (StringUtils.isNotEmpty(prefix)) {
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java
index a2ecb87..8dfe27e 100644
--- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java
+++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java
@@ -18,7 +18,7 @@ package org.apache.dubbo.config;
 
 import org.apache.dubbo.common.Constants;
 import org.apache.dubbo.common.URL;
-import org.apache.dubbo.common.config.CompositeConfiguration;
+import org.apache.dubbo.common.config.Configuration;
 import org.apache.dubbo.common.config.Environment;
 import org.apache.dubbo.common.extension.ExtensionLoader;
 import org.apache.dubbo.common.logger.Logger;
@@ -499,14 +499,14 @@ public abstract class AbstractConfig implements Serializable {
         try {
             Environment env = Environment.getInstance();
             env.addAppConfig(getPrefix(), getId(), getMetaData());
-            CompositeConfiguration compositeConfiguration = env.getStartupCompositeConf(getPrefix(), getId());
+            Configuration configuration = env.getConfiguration(getPrefix(), getId());
 
             // loop methods, get override value and set the new value back to method
             Method[] methods = getClass().getMethods();
             for (Method method : methods) {
                 if (ClassHelper.isSetter(method)) {
                     try {
-                        String value = compositeConfiguration.getString(extractPropertyName(getClass(), method));
+                        String value = configuration.getString(extractPropertyName(getClass(), method));
                         // isTypeMatch() is called to avoid duplicate and incorrect update, for example, we have two 'setGeneric' methods in ReferenceConfig.
                         if (value != null && ClassHelper.isTypeMatch(method.getParameterTypes()[0], value)) {
                             method.invoke(this, ClassHelper.convertPrimitive(method.getParameterTypes()[0], value));
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java
index 69dfc2a..438534d 100644
--- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java
+++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java
@@ -131,7 +131,7 @@ public abstract class AbstractInterfaceConfig extends AbstractMethodConfig {
 
         if (registries == null || registries.isEmpty()) {
             registries = new ArrayList<>();
-            String registryIds = Environment.getInstance().getStartupCompositeConf(null, null).getString("dubbo.registries");
+            String registryIds = Environment.getInstance().getConfiguration(null, null).getString("dubbo.registries");
             if (StringUtils.isNotEmpty(registryIds)) {
                 Arrays.stream(Constants.COMMA_SPLIT_PATTERN.split(registryIds))
                         .map(regId -> {
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java
index 4bc8a4d..0853584 100644
--- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java
+++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java
@@ -733,7 +733,7 @@ public class ServiceConfig<T> extends AbstractServiceConfig {
         // backward compatibility
         if (protocols == null || protocols.isEmpty()) {
             //check 'dubbo.protocols=dubboProtocolId,hessianProtocolId' and decide if we need multiple protocols
-            String protocolIds = Environment.getInstance().getStartupCompositeConf(null, null).getString("dubbo.protocols");
+            String protocolIds = Environment.getInstance().getConfiguration(null, null).getString("dubbo.protocols");
             if (StringUtils.isNotEmpty(protocolIds)) {
                 Arrays.stream(Constants.COMMA_SPLIT_PATTERN.split(protocolIds))
                         .map(pId -> {
diff --git a/dubbo-configcenter/dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/ConfigurationUtils.java b/dubbo-configcenter/dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/ConfigurationUtils.java
index a0c659b..ea8a78d 100644
--- a/dubbo-configcenter/dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/ConfigurationUtils.java
+++ b/dubbo-configcenter/dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/ConfigurationUtils.java
@@ -17,61 +17,27 @@
 package org.apache.dubbo.configcenter;
 
 import org.apache.dubbo.common.Constants;
-import org.apache.dubbo.common.URL;
-import org.apache.dubbo.common.config.CompositeConfiguration;
+import org.apache.dubbo.common.config.Configuration;
 import org.apache.dubbo.common.config.Environment;
-import org.apache.dubbo.common.extension.ExtensionLoader;
 import org.apache.dubbo.common.utils.CollectionUtils;
 
-import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
+
+import static org.apache.dubbo.common.extension.ExtensionLoader.getExtensionLoader;
 
 /**
  * Utilities for manipulating configurations from different sources
  */
 public class ConfigurationUtils {
-    private static final CompositeConfiguration compositeConfiguration;
-
-    static {
-        compositeConfiguration = new CompositeConfiguration();
-        compositeConfiguration.addConfiguration(getDynamicConfiguration());
-        compositeConfiguration.addConfiguration(Environment.getInstance().getAppExternalConfig(null, null));
-        compositeConfiguration.addConfiguration(Environment.getInstance().getExternalConfig(null, null));
-        compositeConfiguration.addConfiguration(Environment.getInstance().getSystemConfig(null, null));
-        compositeConfiguration.addConfiguration(Environment.getInstance().getPropertiesConfig(null, null));
-    }
-
-    private volatile Map<String, CompositeConfiguration> runtimeCompositeConfsHolder = new ConcurrentHashMap<>();
-
-    /**
-     * FIXME This method will recreate Configuration for each RPC, how much latency affect will this action has on performance?
-     *
-     * @param url,    the url metadata.
-     * @param method, the method name the RPC is trying to invoke.
-     * @return
-     */
-    public static CompositeConfiguration getRuntimeCompositeConf(URL url, String method) {
-        CompositeConfiguration compositeConfiguration = new CompositeConfiguration();
-
-        String app = url.getParameter(Constants.APPLICATION_KEY);
-        String service = url.getServiceKey();
-        compositeConfiguration.addConfiguration(new ConfigurationWrapper(app, service, method, getDynamicConfiguration()));
-
-        compositeConfiguration.addConfiguration(url.toConfiguration());
-
-        return compositeConfiguration;
-    }
-
     /**
      * If user opens DynamicConfig, the extension instance must has been created during the initialization of
      * ConfigCenterConfig with the right extension type user specifies. If no DynamicConfig presents,
      * NopDynamicConfiguration will be used.
      */
     public static DynamicConfiguration getDynamicConfiguration() {
-        Set<DynamicConfiguration> configurations = ExtensionLoader.getExtensionLoader(DynamicConfiguration.class).getExtensions();
+        Set<DynamicConfiguration> configurations = getExtensionLoader(DynamicConfiguration.class).getExtensions();
         if (CollectionUtils.isEmpty(configurations)) {
-            return ExtensionLoader.getExtensionLoader(DynamicConfiguration.class).getDefaultExtension();
+            return getExtensionLoader(DynamicConfiguration.class).getDefaultExtension();
         } else {
             return configurations.iterator().next();
         }
@@ -80,7 +46,9 @@ public class ConfigurationUtils {
     @SuppressWarnings("deprecation")
     public static int getServerShutdownTimeout() {
         int timeout = Constants.DEFAULT_SERVER_SHUTDOWN_TIMEOUT;
-        String value = getProperty(Constants.SHUTDOWN_WAIT_KEY);
+        Configuration configuration = Environment.getInstance().getConfiguration();
+        String value = configuration.getString(Constants.SHUTDOWN_WAIT_KEY);
+
         if (value != null && value.length() > 0) {
             try {
                 timeout = Integer.parseInt(value);
@@ -88,7 +56,7 @@ public class ConfigurationUtils {
                 // ignore
             }
         } else {
-            value = getProperty(Constants.SHUTDOWN_WAIT_SECONDS_KEY);
+            value = configuration.getString(Constants.SHUTDOWN_WAIT_SECONDS_KEY);
             if (value != null && value.length() > 0) {
                 try {
                     timeout = Integer.parseInt(value) * 1000;
@@ -99,13 +67,4 @@ public class ConfigurationUtils {
         }
         return timeout;
     }
-
-    public static String getProperty(String key) {
-        return compositeConfiguration.getString(key);
-    }
-
-    public static String getProperty(String key, String defaultValue) {
-        return compositeConfiguration.getString(key, defaultValue);
-    }
-
 }
diff --git a/dubbo-container/dubbo-container-log4j/src/main/java/org/apache/dubbo/container/log4j/Log4jContainer.java b/dubbo-container/dubbo-container-log4j/src/main/java/org/apache/dubbo/container/log4j/Log4jContainer.java
index 78304bb..d7827e9 100644
--- a/dubbo-container/dubbo-container-log4j/src/main/java/org/apache/dubbo/container/log4j/Log4jContainer.java
+++ b/dubbo-container/dubbo-container-log4j/src/main/java/org/apache/dubbo/container/log4j/Log4jContainer.java
@@ -16,7 +16,8 @@
  */
 package org.apache.dubbo.container.log4j;
 
-import org.apache.dubbo.configcenter.ConfigurationUtils;
+import org.apache.dubbo.common.config.Configuration;
+import org.apache.dubbo.common.config.Environment;
 import org.apache.dubbo.container.Container;
 
 import org.apache.log4j.Appender;
@@ -43,9 +44,10 @@ public class Log4jContainer implements Container {
     @Override
     @SuppressWarnings("unchecked")
     public void start() {
-        String file = ConfigurationUtils.getProperty(LOG4J_FILE);
+        Configuration configuration = Environment.getInstance().getConfiguration();
+        String file = configuration.getString(LOG4J_FILE);
         if (file != null && file.length() > 0) {
-            String level = ConfigurationUtils.getProperty(LOG4J_LEVEL);
+            String level = configuration.getString(LOG4J_LEVEL);
             if (level == null || level.length() == 0) {
                 level = DEFAULT_LOG4J_LEVEL;
             }
@@ -59,7 +61,7 @@ public class Log4jContainer implements Container {
             properties.setProperty("log4j.appender.application.layout.ConversionPattern", "%d [%t] %-5p %C{6} (%F:%L) - %m%n");
             PropertyConfigurator.configure(properties);
         }
-        String subdirectory = ConfigurationUtils.getProperty(LOG4J_SUBDIRECTORY);
+        String subdirectory = configuration.getString(LOG4J_SUBDIRECTORY);
         if (subdirectory != null && subdirectory.length() > 0) {
             Enumeration<org.apache.log4j.Logger> ls = LogManager.getCurrentLoggers();
             while (ls.hasMoreElements()) {