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:04 UTC

[incubator-dubbo] branch dev-metadata updated (7adf111 -> 1e28f7a)

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

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


    from 7adf111  Merge pull request  #2818, code review for AbstractConfig.
     new 1bebd2d  Fix problems in Configuration: remove cache usage in startupCompositeConfiguration.
     new 1e28f7a  Merge branch 'dev-metadata' of https://github.com/apache/incubator-dubbo into dev-metadata

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/dubbo/common/config/Environment.java    | 24 ++++++++++++++--------
 .../org/apache/dubbo/config/AbstractConfig.java    |  3 ++-
 .../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, 24 insertions(+), 20 deletions(-)


[incubator-dubbo] 01/02: Fix problems in Configuration: remove cache usage in startupCompositeConfiguration.

Posted by li...@apache.org.
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() {


[incubator-dubbo] 02/02: Merge branch 'dev-metadata' of https://github.com/apache/incubator-dubbo into dev-metadata

Posted by li...@apache.org.
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 1e28f7a49980ad5ef5d373ed11d004de7d7082aa
Merge: 1bebd2d 7adf111
Author: ken.lj <ke...@gmail.com>
AuthorDate: Thu Nov 22 17:29:21 2018 +0800

    Merge branch 'dev-metadata' of https://github.com/apache/incubator-dubbo into dev-metadata

 .../java/org/apache/dubbo/common/Constants.java    |  18 ++-
 .../org/apache/dubbo/common/utils/ClassHelper.java |  55 ++++++++
 .../org/apache/dubbo/config/AbstractConfig.java    | 142 ++++++---------------
 .../dubbo/config/AbstractInterfaceConfig.java      |  18 ++-
 .../apache/dubbo/config/MetadataReportConfig.java  |  16 ++-
 .../apache/dubbo/config/RegistryDataConfig.java    |  35 +++--
 .../apache/dubbo/config/utils/ConfigConverter.java |  40 ------
 .../dubbo/configcenter/ConfigurationListener.java  |   5 -
 .../support/apollo/ApolloDynamicConfiguration.java |  73 +++++------
 .../archaius/ArchaiusDynamicConfiguration.java     |  40 +++---
 .../sources/ZooKeeperConfigurationSource.java      |  36 +++---
 .../metadata/identifier/MetadataIdentifier.java    |  13 +-
 .../metadata/support/AbstractMetadataReport.java   |  29 ++---
 .../store/test/JTestMetadataReport4Test.java       |   3 +-
 .../support/AbstractMetadataReportTest.java        |  20 +--
 .../store/redis/RedisMetadataReportTest.java       |   9 +-
 .../store/zookeeper/ZookeeperMetadataReport.java   |   2 +-
 .../zookeeper/ZookeeperMetadataReportTest.java     |   2 +-
 .../registry/integration/RegistryProtocol.java     |   5 -
 19 files changed, 263 insertions(+), 298 deletions(-)

diff --cc dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java
index 43a327b,4395a71..91e226d
--- 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
@@@ -552,15 -491,16 +491,17 @@@ public abstract class AbstractConfig im
              // loop methods, get override value and set the new value back to method
              Method[] methods = getClass().getMethods();
              for (Method method : methods) {
-                 if (isSetter(method)) {
+                 if (ClassHelper.isSetter(method)) {
                      try {
-                         String value = compositeConfiguration.getString(extractPropertyName(method));
+                         String value = compositeConfiguration.getString(extractPropertyName(getClass(), 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));
+                             method.invoke(this, ClassHelper.convertPrimitive(method.getParameterTypes()[0], value));
                          }
                      } catch (NoSuchMethodException e) {
-                         logger.info("Failed to override the property " + method.getName() + " in " + this.getClass().getSimpleName() + ", please make sure every property has a getter/setter pair.");
+                         logger.info("Failed to override the property " + method.getName() + " in " +
+                                 this.getClass().getSimpleName() +
+                                 ", please make sure every property has getter/setter method provided.");
                      }
                  }
              }
diff --cc dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java
index 5460861,0d484a7..69dfc2a
--- 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
@@@ -229,10 -235,14 +236,11 @@@ public abstract class AbstractInterface
      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();
+             Map<String, String> registryDataConfigurationMap = new HashMap<>(4);
+             appendParameters(registryDataConfigurationMap, registryDataConfig);
              for (RegistryConfig config : registries) {
                  String address = config.getAddress();
                  if (address == null || address.length() == 0) {