Refactor, configuration override does not work properly. (apache#5709)

Author: chickenlj

dubbo-common/src/main/java/org/apache/dubbo/common/config/AbstractPrefixConfiguration.java

@@ -18,27 +18,41 @@
 
 import org.apache.dubbo.common.logger.Logger;
 import org.apache.dubbo.common.logger.LoggerFactory;
+import org.apache.dubbo.common.utils.StringUtils;
 
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 
 /**
- *
+ * This is an abstraction specially customized for the sequence Dubbo retrieves properties.
  */
 public class CompositeConfiguration implements Configuration {
     private Logger logger = LoggerFactory.getLogger(CompositeConfiguration.class);
 
+    private String id;
+    private String prefix;
+
     /**
      * List holding all the configuration
      */
     private List<Configuration> configList = new LinkedList<Configuration>();
 
     public CompositeConfiguration() {
+        this(null, null);
+    }
 
+    public CompositeConfiguration(String prefix, String id) {
+        if (StringUtils.isNotEmpty(prefix) && !prefix.endsWith(".")) {
+            this.prefix = prefix + ".";
+        } else {
+            this.prefix = prefix;
+        }
+        this.id = id;
     }
 
     public CompositeConfiguration(Configuration... configurations) {
+        this();
         if (configurations != null && configurations.length > 0) {
             Arrays.stream(configurations).filter(config -> !configList.contains(config)).forEach(configList::add);
         }
@@ -83,4 +97,20 @@ public Object getInternalProperty(String key) {
     public boolean containsKey(String key) {
         return configList.stream().anyMatch(c -> c.containsKey(key));
     }
+
+    @Override
+    public Object getProperty(String key, Object defaultValue) {
+        Object value = null;
+        if (StringUtils.isNotEmpty(prefix)) {
+            if (StringUtils.isNotEmpty(id)) {
+                value = getInternalProperty(prefix + id + "." + key);
+            }
+            if (value == null) {
+                value = getInternalProperty(prefix + key);
+            }
+        } else {
+            value = getInternalProperty(key);
+        }
+        return value != null ? value : defaultValue;
+    }
 }

dubbo-common/src/main/java/org/apache/dubbo/common/config/CompositeConfiguration.java

@@ -17,12 +17,12 @@
 package org.apache.dubbo.common.config;
 
 import org.apache.dubbo.common.config.configcenter.DynamicConfiguration;
-import org.apache.dubbo.common.constants.CommonConstants;
 import org.apache.dubbo.common.context.FrameworkExt;
 import org.apache.dubbo.common.context.LifecycleAdapter;
 import org.apache.dubbo.common.extension.DisableInject;
-import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.config.AbstractConfig;
 import org.apache.dubbo.config.ConfigCenterConfig;
+import org.apache.dubbo.config.context.ConfigConfigurationAdapter;
 import org.apache.dubbo.config.context.ConfigManager;
 import org.apache.dubbo.rpc.model.ApplicationModel;
 
@@ -31,15 +31,19 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 public class Environment extends LifecycleAdapter implements FrameworkExt {
     public static final String NAME = "environment";
 
-    private Map<String, PropertiesConfiguration> propertiesConfigs = new ConcurrentHashMap<>();
-    private Map<String, SystemConfiguration> systemConfigs = new ConcurrentHashMap<>();
-    private Map<String, EnvironmentConfiguration> environmentConfigs = new ConcurrentHashMap<>();
-    private Map<String, InmemoryConfiguration> externalConfigs = new ConcurrentHashMap<>();
-    private Map<String, InmemoryConfiguration> appExternalConfigs = new ConcurrentHashMap<>();
+    private final PropertiesConfiguration propertiesConfiguration;
+    private final SystemConfiguration systemConfiguration;
+    private final EnvironmentConfiguration environmentConfiguration;
+    private final InmemoryConfiguration externalConfiguration;
+    private final InmemoryConfiguration appExternalConfiguration;
+
+    private final ConcurrentMap<AbstractConfig, CompositeConfiguration> prefixedConfigurations = new ConcurrentHashMap<>();
+    private CompositeConfiguration globalConfiguration;
 
     private Map<String, String> externalConfigurationMap = new HashMap<>();
     private Map<String, String> appExternalConfigurationMap = new HashMap<>();
@@ -48,6 +52,14 @@ public class Environment extends LifecycleAdapter implements FrameworkExt {
 
     private DynamicConfiguration dynamicConfiguration;
 
+    public Environment() {
+        this.propertiesConfiguration = new PropertiesConfiguration();
+        this.systemConfiguration = new SystemConfiguration();
+        this.environmentConfiguration = new EnvironmentConfiguration();
+        this.externalConfiguration = new InmemoryConfiguration();
+        this.appExternalConfiguration = new InmemoryConfiguration();
+    }
+
     @Override
     public void initialize() throws IllegalStateException {
         ConfigManager configManager = ApplicationModel.getConfigManager();
@@ -58,34 +70,9 @@ public void initialize() throws IllegalStateException {
                 this.setAppExternalConfigMap(config.getAppExternalConfiguration());
             }
         });
-    }
 
-    public PropertiesConfiguration getPropertiesConfig(String prefix, String id) {
-        return propertiesConfigs.computeIfAbsent(toKey(prefix, id), k -> new PropertiesConfiguration(prefix, id));
-    }
-
-    public SystemConfiguration getSystemConfig(String prefix, String id) {
-        return systemConfigs.computeIfAbsent(toKey(prefix, id), k -> new SystemConfiguration(prefix, id));
-    }
-
-    public InmemoryConfiguration getExternalConfig(String prefix, String id) {
-        return externalConfigs.computeIfAbsent(toKey(prefix, id), k -> {
-            InmemoryConfiguration configuration = new InmemoryConfiguration(prefix, id);
-            configuration.setProperties(externalConfigurationMap);
-            return configuration;
-        });
-    }
-
-    public InmemoryConfiguration getAppExternalConfig(String prefix, String id) {
-        return appExternalConfigs.computeIfAbsent(toKey(prefix, id), k -> {
-            InmemoryConfiguration configuration = new InmemoryConfiguration(prefix, id);
-            configuration.setProperties(appExternalConfigurationMap);
-            return configuration;
-        });
-    }
-
-    public EnvironmentConfiguration getEnvironmentConfig(String prefix, String id) {
-        return environmentConfigs.computeIfAbsent(toKey(prefix, id), k -> new EnvironmentConfiguration(prefix, id));
+        this.externalConfiguration.setProperties(externalConfigurationMap);
+        this.appExternalConfiguration.setProperties(appExternalConfigurationMap);
     }
 
     @DisableInject
@@ -119,46 +106,65 @@ public void updateAppExternalConfigurationMap(Map<String, String> externalMap) {
     }
 
     /**
-     * Create new instance for each call, since it will be called only at startup, I think there's no big deal of the potential cost.
-     * Otherwise, if use cache, we should make sure each Config has a unique id which is difficult to guarantee because is on the user's side,
-     * especially when it comes to ServiceConfig and ReferenceConfig.
+     * At start-up, Dubbo is driven by various configuration, such as Application, Registry, Protocol, etc.
+     * All configurations will be converged into a data bus - URL, and then drive the subsequent process.
+     * <p>
+     * At present, there are many configuration sources, including AbstractConfig (API, XML, annotation), - D, config center, etc.
+     * This method helps us to filter out the most priority values from various configuration sources.
      *
-     * @param prefix
-     * @param id
+     * @param config
      * @return
      */
-    public CompositeConfiguration getConfiguration(String prefix, String id) {
-        CompositeConfiguration compositeConfiguration = new CompositeConfiguration();
-        // Config center has the highest priority
-        compositeConfiguration.addConfiguration(this.getSystemConfig(prefix, id));
-        compositeConfiguration.addConfiguration(this.getEnvironmentConfig(prefix, id));
-        compositeConfiguration.addConfiguration(this.getAppExternalConfig(prefix, id));
-        compositeConfiguration.addConfiguration(this.getExternalConfig(prefix, id));
-        compositeConfiguration.addConfiguration(this.getPropertiesConfig(prefix, id));
-        return compositeConfiguration;
+    public synchronized CompositeConfiguration getPrefixedConfiguration(AbstractConfig config) {
+//        CompositeConfiguration prefixedConfiguration =
+//                prefixedConfigurations.putIfAbsent(config, new CompositeConfiguration(config.getPrefix(), config.getId()));
+//        if (prefixedConfiguration != null) {
+//            return prefixedConfiguration;
+//        }
+//        prefixedConfiguration = prefixedConfigurations.get(config);
+        CompositeConfiguration prefixedConfiguration = new CompositeConfiguration(config.getPrefix(), config.getId());
+        Configuration configuration = new ConfigConfigurationAdapter(config);
+        if (this.isConfigCenterFirst()) {
+            // The sequence would be: SystemConfiguration -> AppExternalConfiguration -> ExternalConfiguration -> AbstractConfig -> PropertiesConfiguration
+            // Config center has the highest priority
+            prefixedConfiguration.addConfiguration(systemConfiguration);
+            prefixedConfiguration.addConfiguration(environmentConfiguration);
+            prefixedConfiguration.addConfiguration(appExternalConfiguration);
+            prefixedConfiguration.addConfiguration(externalConfiguration);
+            prefixedConfiguration.addConfiguration(configuration);
+            prefixedConfiguration.addConfiguration(propertiesConfiguration);
+        } else {
+            // The sequence would be: SystemConfiguration -> AbstractConfig -> AppExternalConfiguration -> ExternalConfiguration -> PropertiesConfiguration
+            // Config center has the highest priority
+            prefixedConfiguration.addConfiguration(systemConfiguration);
+            prefixedConfiguration.addConfiguration(environmentConfiguration);
+            prefixedConfiguration.addConfiguration(configuration);
+            prefixedConfiguration.addConfiguration(appExternalConfiguration);
+            prefixedConfiguration.addConfiguration(externalConfiguration);
+            prefixedConfiguration.addConfiguration(propertiesConfiguration);
+        }
+        return prefixedConfiguration;
     }
 
+    /**
+     * There are two ways to get configuration during exposure / reference or at runtime:
+     * 1. URL, The value in the URL is relatively fixed. we can get value directly.
+     * 2. The configuration exposed in this method is convenient for us to query the latest values from multiple
+     * prioritized sources, it also guarantees that configs changed dynamically can take effect on the fly.
+     */
     public Configuration getConfiguration() {
-        return getConfiguration(null, null);
-    }
-
-    private static String toKey(String prefix, String id) {
-        StringBuilder sb = new StringBuilder();
-        if (StringUtils.isNotEmpty(prefix)) {
-            sb.append(prefix);
-        }
-        if (StringUtils.isNotEmpty(id)) {
-            sb.append(id);
-        }
-
-        if (sb.length() > 0 && sb.charAt(sb.length() - 1) != '.') {
-            sb.append(".");
-        }
-
-        if (sb.length() > 0) {
-            return sb.toString();
+        if (globalConfiguration == null) {
+            globalConfiguration = new CompositeConfiguration();
+            if (dynamicConfiguration != null) {
+                globalConfiguration.addConfiguration(dynamicConfiguration);
+            }
+            globalConfiguration.addConfiguration(systemConfiguration);
+            globalConfiguration.addConfiguration(environmentConfiguration);
+            globalConfiguration.addConfiguration(appExternalConfiguration);
+            globalConfiguration.addConfiguration(externalConfiguration);
+            globalConfiguration.addConfiguration(propertiesConfiguration);
         }
-        return CommonConstants.DUBBO;
+        return globalConfiguration;
     }
 
     public boolean isConfigCenterFirst() {
@@ -187,13 +193,13 @@ public void destroy() throws IllegalStateException {
 
     // For test
     public void clearExternalConfigs() {
-        this.externalConfigs.clear();
+        this.externalConfiguration.clear();
         this.externalConfigurationMap.clear();
     }
 
     // For test
     public void clearAppExternalConfigs() {
-        this.appExternalConfigs.clear();
+        this.appExternalConfiguration.clear();
         this.appExternalConfigurationMap.clear();
     }
 }

dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java

@@ -21,15 +21,7 @@
 /**
  * Configuration from system environment
  */
-public class EnvironmentConfiguration extends AbstractPrefixConfiguration {
-
-    public EnvironmentConfiguration(String prefix, String id) {
-        super(prefix, id);
-    }
-
-    public EnvironmentConfiguration() {
-        this(null, null);
-    }
+public class EnvironmentConfiguration implements Configuration {
 
     @Override
     public Object getInternalProperty(String key) {

dubbo-common/src/main/java/org/apache/dubbo/common/config/EnvironmentConfiguration.java

@@ -22,19 +22,11 @@
 /**
  * In-memory configuration
  */
-public class InmemoryConfiguration extends AbstractPrefixConfiguration {
+public class InmemoryConfiguration implements Configuration {
 
     // stores the configuration key-value pairs
     private Map<String, String> store = new LinkedHashMap<>();
 
-    public InmemoryConfiguration(String prefix, String id) {
-        super(prefix, id);
-    }
-
-    public InmemoryConfiguration() {
-        this(null, null);
-    }
-
     @Override
     public Object getInternalProperty(String key) {
         return store.get(key);
@@ -64,4 +56,9 @@ public void setProperties(Map<String, String> properties) {
             this.store = properties;
         }
     }
+
+    // for unit test
+    public void clear() {
+        this.store.clear();
+    }
 }

dubbo-common/src/main/java/org/apache/dubbo/common/config/InmemoryConfiguration.java

@@ -27,11 +27,9 @@
 /**
  * Configuration from system properties and dubbo.properties
  */
-public class PropertiesConfiguration extends AbstractPrefixConfiguration {
-
-    public PropertiesConfiguration(String prefix, String id) {
-        super(prefix, id);
+public class PropertiesConfiguration implements Configuration {
 
+    public PropertiesConfiguration() {
         ExtensionLoader<OrderedPropertiesProvider> propertiesProviderExtensionLoader = ExtensionLoader.getExtensionLoader(OrderedPropertiesProvider.class);
         Set<String> propertiesProviderNames = propertiesProviderExtensionLoader.getSupportedExtensions();
         if (propertiesProviderNames == null || propertiesProviderNames.isEmpty()) {
@@ -59,10 +57,6 @@ public PropertiesConfiguration(String prefix, String id) {
         ConfigUtils.setProperties(properties);
     }
 
-    public PropertiesConfiguration() {
-        this(null, null);
-    }
-
     @Override
     public Object getInternalProperty(String key) {
         return ConfigUtils.getProperty(key);