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/22 09:30:05 UTC
[incubator-dubbo] 01/02: Fix problems in Configuration: remove
cache usage in startupCompositeConfiguration.
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
commit 1bebd2d28daa4573a0edecbf4b9d871827d5d9f0
Author: ken.lj <ke...@gmail.com>
AuthorDate: Thu Nov 22 17:23:27 2018 +0800
Fix problems in Configuration: remove cache usage in startupCompositeConfiguration.
---
.../apache/dubbo/common/config/Environment.java | 24 ++++++++++++++--------
.../org/apache/dubbo/config/AbstractConfig.java | 20 ++++++++++++++++--
.../dubbo/config/AbstractInterfaceConfig.java | 9 +++-----
.../org/apache/dubbo/config/ReferenceConfig.java | 2 +-
.../org/apache/dubbo/config/ServiceConfig.java | 2 +-
.../dubbo/config/spring/ConfigCenterBean.java | 2 +-
.../dubbo/configcenter/ConfigurationUtils.java | 2 +-
7 files changed, 40 insertions(+), 21 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 efc77d6..1d94c9f 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
@@ -34,7 +34,6 @@ public class Environment {
private Map<String, EnvironmentConfiguration> environmentConfsHolder = new ConcurrentHashMap<>();
private Map<String, InmemoryConfiguration> externalConfsHolder = new ConcurrentHashMap<>();
private Map<String, InmemoryConfiguration> appExternalConfsHolder = new ConcurrentHashMap<>();
- private Map<String, CompositeConfiguration> startupCompositeConfsHolder = new ConcurrentHashMap<>();
private boolean isConfigCenterFirst = true;
@@ -89,15 +88,22 @@ public class Environment {
this.appExternalConfigurationMap.putAll(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.
+ *
+ * @param prefix
+ * @param id
+ * @return
+ */
public CompositeConfiguration getStartupCompositeConf(String prefix, String id) {
- return startupCompositeConfsHolder.computeIfAbsent(toKey(prefix, id), k -> {
- 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));
- return compositeConfiguration;
- });
+ 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));
+ return compositeConfiguration;
}
private static String toKey(String keypart1, String keypart2) {
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 a894d76..43a327b 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
@@ -302,6 +302,21 @@ public abstract class AbstractConfig implements Serializable {
return value;
}
+ /**
+ * 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
@@ -540,7 +555,8 @@ public abstract class AbstractConfig implements Serializable {
if (isSetter(method)) {
try {
String value = compositeConfiguration.getString(extractPropertyName(method));
- if (value != null) {
+ // isTypeMatch() is called to avoid duplicate and incorrect update, for example, we have two 'setGeneric' methods in ReferenceConfig.
+ if (value != null && isTypeMatch(method.getParameterTypes()[0], value)) {
method.invoke(this, convertPrimitive(method.getParameterTypes()[0], value));
}
} catch (NoSuchMethodException e) {
@@ -564,7 +580,7 @@ public abstract class AbstractConfig implements Serializable {
return false;
}
- public String extractPropertyName(Method setter) throws Exception {
+ private String extractPropertyName(Method setter) throws Exception {
String propertyName = setter.getName().substring("set".length());
Method getter = null;
try {
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 b1f7223..5460861 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
@@ -32,6 +32,7 @@ import org.apache.dubbo.configcenter.DynamicConfiguration;
import org.apache.dubbo.metadata.integration.MetadataReportService;
import org.apache.dubbo.monitor.MonitorFactory;
import org.apache.dubbo.monitor.MonitorService;
+import org.apache.dubbo.registry.RegistryService;
import org.apache.dubbo.rpc.Filter;
import org.apache.dubbo.rpc.InvokerListener;
import org.apache.dubbo.rpc.ProxyFactory;
@@ -225,13 +226,10 @@ public abstract class AbstractInterfaceConfig extends AbstractMethodConfig {
}
}
- protected void checkConfigCenter() {
-
- }
-
protected List<URL> loadRegistries(boolean provider) {
// check && override if necessary
checkRegistry();
+ checkRegistryDataConfig();
List<URL> registryList = new ArrayList<URL>();
if (registries != null && !registries.isEmpty()) {
Map<String, String> registryDataConfigurationMap = this.registryDataConfig.transferToMap();
@@ -244,8 +242,7 @@ public abstract class AbstractInterfaceConfig extends AbstractMethodConfig {
Map<String, String> map = new HashMap<String, String>();
appendParameters(map, application);
appendParameters(map, config);
- //TODO temporarily use literal value to avoid depending on dubbo-registry-api module.
- map.put("path", "org.apache.dubbo.registry.RegistryService");
+ map.put("path", RegistryService.class.getName());
map.put("dubbo", Version.getProtocolVersion());
map.put(Constants.TIMESTAMP_KEY, String.valueOf(System.currentTimeMillis()));
if (ConfigUtils.getPid() > 0) {
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
index 0d93d41..23ad4c8 100644
--- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
+++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
@@ -555,7 +555,7 @@ public class ReferenceConfig<T> extends AbstractReferenceConfig {
@Override
@Parameter(excluded = true)
public String getPrefix() {
- return Constants.DUBBO + ".reference." + interfaceName;
+ return Constants.DUBBO + ".reference";
}
}
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 bc2c20d..4bc8a4d 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
@@ -898,6 +898,6 @@ public class ServiceConfig<T> extends AbstractServiceConfig {
@Override
@Parameter(excluded = true)
public String getPrefix() {
- return Constants.DUBBO + ".service." + interfaceName;
+ return Constants.DUBBO + ".service";
}
}
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 9ce2f46..6c8136f 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
@@ -46,7 +46,7 @@ public class ConfigCenterBean extends ConfigCenterConfig implements Initializing
private transient ApplicationContext applicationContext;
- private boolean auto = false;
+ private Boolean auto = false;
@Override
public void setApplicationContext(ApplicationContext applicationContext) {
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 ef3e6d6..5068c63 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
@@ -65,7 +65,7 @@ public class ConfigurationUtils {
/**
* If user opens DynamicConfig, the extension instance must has been created during the initialization of
- * ConfigCenterConfig with the right extension type user specified. If no DynamicConfig presents,
+ * ConfigCenterConfig with the right extension type user specifies. If no DynamicConfig presents,
* NopDynamicConfiguration will be used.
*/
public static DynamicConfiguration getDynamicConfiguration() {