You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by il...@apache.org on 2018/11/23 02:20:39 UTC
[incubator-dubbo] branch dev-metadata updated: Code review around
Environment, AbstractConfig,
and AbstractPrefixConfiguration's impls. (#2820)
This is an automated email from the ASF dual-hosted git repository.
iluo 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 3dfa8d2 Code review around Environment, AbstractConfig, and AbstractPrefixConfiguration's impls. (#2820)
3dfa8d2 is described below
commit 3dfa8d24e4c0ca6f39c5257c06c8524418a33229
Author: Ian Luo <ia...@gmail.com>
AuthorDate: Fri Nov 23 10:20:32 2018 +0800
Code review around Environment, AbstractConfig, and AbstractPrefixConfiguration's impls. (#2820)
* add javadoc, comments, and FIXME
* move appconfig logic into Environment from refresh method
* enhance naming in Environment
* introduce calculatePrefix to avoid dup code
* roll back calculatePrefix().
---
.../apache/dubbo/common/config/Environment.java | 77 ++++++++++++++--------
.../common/config/EnvironmentConfiguration.java | 2 +-
.../dubbo/common/config/InmemoryConfiguration.java | 14 ++--
.../common/config/PropertiesConfiguration.java | 3 +
.../dubbo/common/config/SystemConfiguration.java | 5 +-
.../org/apache/dubbo/config/AbstractConfig.java | 24 +++++--
.../dubbo/config/spring/ConfigCenterBean.java | 4 +-
.../dubbo/configcenter/ConfigurationUtils.java | 8 +--
8 files changed, 86 insertions(+), 51 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 1d94c9f..a6ce939 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
@@ -29,13 +29,14 @@ import java.util.concurrent.ConcurrentHashMap;
public class Environment {
private static final Environment INSTANCE = new Environment();
- private Map<String, PropertiesConfiguration> propertiesConfsHolder = new ConcurrentHashMap<>();
- private Map<String, SystemConfiguration> systemConfsHolder = new ConcurrentHashMap<>();
- private Map<String, EnvironmentConfiguration> environmentConfsHolder = new ConcurrentHashMap<>();
- private Map<String, InmemoryConfiguration> externalConfsHolder = new ConcurrentHashMap<>();
- private Map<String, InmemoryConfiguration> appExternalConfsHolder = new ConcurrentHashMap<>();
+ 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 Map<String, InmemoryConfiguration> appConfigs = new ConcurrentHashMap<>();
- private boolean isConfigCenterFirst = true;
+ private boolean configCenterFirst = true;
private Map<String, String> externalConfigurationMap = new HashMap<>();
private Map<String, String> appExternalConfigurationMap = new HashMap<>();
@@ -44,42 +45,54 @@ public class Environment {
return INSTANCE;
}
- public PropertiesConfiguration getPropertiesConf(String prefix, String id) {
- return propertiesConfsHolder.computeIfAbsent(toKey(prefix, id), k -> new PropertiesConfiguration(prefix, id));
+ public PropertiesConfiguration getPropertiesConfig(String prefix, String id) {
+ return propertiesConfigs.computeIfAbsent(toKey(prefix, id), k -> new PropertiesConfiguration(prefix, id));
}
- public SystemConfiguration getSystemConf(String prefix, String id) {
- return systemConfsHolder.computeIfAbsent(toKey(prefix, id), k -> new SystemConfiguration(prefix, id));
+ public SystemConfiguration getSystemConfig(String prefix, String id) {
+ return systemConfigs.computeIfAbsent(toKey(prefix, id), k -> new SystemConfiguration(prefix, id));
}
- public InmemoryConfiguration getExternalConfiguration(String prefix, String id) {
- return externalConfsHolder.computeIfAbsent(toKey(prefix, id), k -> {
+ public InmemoryConfiguration getExternalConfig(String prefix, String id) {
+ return externalConfigs.computeIfAbsent(toKey(prefix, id), k -> {
InmemoryConfiguration configuration = new InmemoryConfiguration(prefix, id);
configuration.addProperties(externalConfigurationMap);
return configuration;
});
}
- public InmemoryConfiguration getAppExternalConfiguration(String prefix, String id) {
- return appExternalConfsHolder.computeIfAbsent(toKey(prefix, id), k -> {
+ public InmemoryConfiguration getAppExternalConfig(String prefix, String id) {
+ return appExternalConfigs.computeIfAbsent(toKey(prefix, id), k -> {
InmemoryConfiguration configuration = new InmemoryConfiguration(prefix, id);
configuration.addProperties(appExternalConfigurationMap);
return configuration;
});
}
- public EnvironmentConfiguration getEnvironmentConf(String prefix, String id) {
- return environmentConfsHolder.computeIfAbsent(toKey(prefix, id), k -> new EnvironmentConfiguration(prefix, id));
+ public EnvironmentConfiguration getEnvironmentConfig(String prefix, String id) {
+ return environmentConfigs.computeIfAbsent(toKey(prefix, id), k -> new EnvironmentConfiguration(prefix, id));
}
- public synchronized void setExternalConfiguration(Map<String, String> externalConfiguration) {
+ public InmemoryConfiguration getAppConfig(String prefix, String id) {
+ return appConfigs.get(toKey(prefix, id));
+ }
+
+ public synchronized void setExternalConfig(Map<String, String> externalConfiguration) {
this.externalConfigurationMap = externalConfiguration;
}
- public synchronized void setAppExternalConfiguration(Map<String, String> appExternalConfiguration) {
+ public synchronized void setAppExternalConfig(Map<String, String> appExternalConfiguration) {
this.appExternalConfigurationMap = appExternalConfiguration;
}
+ public void addAppConfig(String prefix, String id, Map<String, String> properties) {
+ appConfigs.computeIfAbsent(toKey(prefix, id), k -> {
+ InmemoryConfiguration configuration = new InmemoryConfiguration(prefix, id);
+ configuration.addProperties(properties);
+ return configuration;
+ });
+ }
+
public void updateExternalConfigurationMap(Map<String, String> externalMap) {
this.externalConfigurationMap.putAll(externalMap);
}
@@ -99,20 +112,26 @@ public class Environment {
*/
public CompositeConfiguration getStartupCompositeConf(String prefix, String id) {
CompositeConfiguration compositeConfiguration = new CompositeConfiguration();
- compositeConfiguration.addConfiguration(this.getSystemConf(prefix, id));
- compositeConfiguration.addConfiguration(this.getAppExternalConfiguration(prefix, id));
- compositeConfiguration.addConfiguration(this.getExternalConfiguration(prefix, id));
- compositeConfiguration.addConfiguration(this.getPropertiesConf(prefix, id));
+ compositeConfiguration.addConfiguration(this.getSystemConfig(prefix, id));
+ compositeConfiguration.addConfiguration(this.getAppExternalConfig(prefix, id));
+ compositeConfiguration.addConfiguration(this.getExternalConfig(prefix, id));
+ compositeConfiguration.addConfiguration(this.getPropertiesConfig(prefix, id));
+
+ InmemoryConfiguration appConfig = this.getAppConfig(prefix, id);
+ if (appConfig != null) {
+ int index = configCenterFirst ? 3 : 1;
+ compositeConfiguration.addConfiguration(index, appConfig);
+ }
return compositeConfiguration;
}
- private static String toKey(String keypart1, String keypart2) {
+ private static String toKey(String prefix, String id) {
StringBuilder sb = new StringBuilder();
- if (StringUtils.isNotEmpty(keypart1)) {
- sb.append(keypart1);
+ if (StringUtils.isNotEmpty(prefix)) {
+ sb.append(prefix);
}
- if (StringUtils.isNotEmpty(keypart2)) {
- sb.append(keypart2);
+ if (StringUtils.isNotEmpty(id)) {
+ sb.append(id);
}
if (sb.length() > 0 && sb.charAt(sb.length() - 1) != '.') {
@@ -126,10 +145,10 @@ public class Environment {
}
public boolean isConfigCenterFirst() {
- return isConfigCenterFirst;
+ return configCenterFirst;
}
public void setConfigCenterFirst(boolean configCenterFirst) {
- isConfigCenterFirst = configCenterFirst;
+ this.configCenterFirst = configCenterFirst;
}
}
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/config/EnvironmentConfiguration.java b/dubbo-common/src/main/java/org/apache/dubbo/common/config/EnvironmentConfiguration.java
index a38e831..0f6f00a 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/config/EnvironmentConfiguration.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/config/EnvironmentConfiguration.java
@@ -17,7 +17,7 @@
package org.apache.dubbo.common.config;
/**
- *
+ * Configuration from system environment
*/
public class EnvironmentConfiguration extends AbstractPrefixConfiguration {
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/config/InmemoryConfiguration.java b/dubbo-common/src/main/java/org/apache/dubbo/common/config/InmemoryConfiguration.java
index a54e941..b4a9c48 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/config/InmemoryConfiguration.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/config/InmemoryConfiguration.java
@@ -20,13 +20,11 @@ import java.util.LinkedHashMap;
import java.util.Map;
/**
- *
+ * In-memory configuration
*/
public class InmemoryConfiguration extends AbstractPrefixConfiguration {
- /**
- * stores the configuration key-value pairs
- */
+ // stores the configuration key-value pairs
private Map<String, String> store = new LinkedHashMap<>();
public InmemoryConfiguration(String prefix, String id) {
@@ -43,15 +41,15 @@ public class InmemoryConfiguration extends AbstractPrefixConfiguration {
}
/**
- * Replace previous value if there is one
- *
- * @param key
- * @param value
+ * Add one property into the store, the previous value will be replaced if the key exists
*/
public void addProperty(String key, String value) {
store.put(key, value);
}
+ /**
+ * Add a set of properties into the store
+ */
public void addProperties(Map<String, String> properties) {
store.putAll(properties);
}
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/config/PropertiesConfiguration.java b/dubbo-common/src/main/java/org/apache/dubbo/common/config/PropertiesConfiguration.java
index 3a11c31..d76c3a1 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/config/PropertiesConfiguration.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/config/PropertiesConfiguration.java
@@ -20,6 +20,9 @@ import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.utils.ConfigUtils;
+/**
+ * Configuration from system properties and dubbo.properties
+ */
public class PropertiesConfiguration extends AbstractPrefixConfiguration {
private static final Logger logger = LoggerFactory.getLogger(PropertiesConfiguration.class);
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/config/SystemConfiguration.java b/dubbo-common/src/main/java/org/apache/dubbo/common/config/SystemConfiguration.java
index 853fdf2..2a5bb54 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/config/SystemConfiguration.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/config/SystemConfiguration.java
@@ -16,8 +16,11 @@
*/
package org.apache.dubbo.common.config;
+
/**
- *
+ * FIXME: is this really necessary? PropertiesConfiguration should have already covered this:
+ * @see PropertiesConfiguration
+ * @See ConfigUtils#getProperty(String)
*/
public class SystemConfiguration extends AbstractPrefixConfiguration {
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 91e226d..1cb3a30 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
@@ -20,7 +20,6 @@ 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.Environment;
-import org.apache.dubbo.common.config.InmemoryConfiguration;
import org.apache.dubbo.common.extension.ExtensionLoader;
import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
@@ -256,6 +255,21 @@ public abstract class AbstractConfig implements Serializable {
}
}
+ /**
+ * We only check boolean value at this moment.
+ *
+ * @param type
+ * @param value
+ * @return
+ */
+ private static boolean isTypeMatch(Class<?> type, String value) {
+ if ((type == boolean.class || type == Boolean.class)
+ && !("true".equals(value) || "false".equals(value))) {
+ return false;
+ }
+ return true;
+ }
+
protected static void checkExtension(Class<?> type, String property, String value) {
checkName(property, value);
if (value != null && value.length() > 0
@@ -482,11 +496,9 @@ public abstract class AbstractConfig implements Serializable {
init = true;
try {
- InmemoryConfiguration configuration = new InmemoryConfiguration(getPrefix(), getId());
- configuration.addProperties(getMetaData());
- CompositeConfiguration compositeConfiguration = Environment.getInstance().getStartupCompositeConf(getPrefix(), getId());
- int index = Environment.getInstance().isConfigCenterFirst() ? 3 : 1;
- compositeConfiguration.addConfiguration(index, configuration);
+ Environment env = Environment.getInstance();
+ env.addAppConfig(getPrefix(), getId(), getMetaData());
+ CompositeConfiguration compositeConfiguration = env.getStartupCompositeConf(getPrefix(), getId());
// loop methods, get override value and set the new value back to method
Method[] methods = getClass().getMethods();
diff --git a/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/ConfigCenterBean.java b/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/ConfigCenterBean.java
index 6c8136f..6b5782e 100644
--- a/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/ConfigCenterBean.java
+++ b/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/ConfigCenterBean.java
@@ -107,8 +107,8 @@ public class ConfigCenterBean extends ConfigCenterConfig implements Initializing
if (auto) {
Map<String, String> externalProperties = getConfigurations(getConfigfile(), environment);
Map<String, String> appExternalProperties = getConfigurations("application." + getConfigfile(), environment);
- org.apache.dubbo.common.config.Environment.getInstance().setExternalConfiguration(externalProperties);
- org.apache.dubbo.common.config.Environment.getInstance().setAppExternalConfiguration(appExternalProperties);
+ org.apache.dubbo.common.config.Environment.getInstance().setExternalConfig(externalProperties);
+ org.apache.dubbo.common.config.Environment.getInstance().setAppExternalConfig(appExternalProperties);
this.init();
}
}
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 5068c63..a0c659b 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
@@ -36,10 +36,10 @@ public class ConfigurationUtils {
static {
compositeConfiguration = new CompositeConfiguration();
compositeConfiguration.addConfiguration(getDynamicConfiguration());
- compositeConfiguration.addConfiguration(Environment.getInstance().getAppExternalConfiguration(null, null));
- compositeConfiguration.addConfiguration(Environment.getInstance().getExternalConfiguration(null, null));
- compositeConfiguration.addConfiguration(Environment.getInstance().getSystemConf(null, null));
- compositeConfiguration.addConfiguration(Environment.getInstance().getPropertiesConf(null, null));
+ 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<>();