You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/07/19 03:17:45 UTC

[GitHub] [dubbo] kylixs commented on a change in pull request #8308: support dubbo.registry.parameters.item3=value3 configuration properties

kylixs commented on a change in pull request #8308:
URL: https://github.com/apache/dubbo/pull/8308#discussion_r671960777



##########
File path: dubbo-common/src/main/java/org/apache/dubbo/config/AbstractConfig.java
##########
@@ -540,10 +541,10 @@ public void refresh() {
                     String propertyName = extractPropertyName(method.getName());
                     String value = StringUtils.trim(subPropsConfiguration.getString(propertyName));
                     if (StringUtils.hasText(value)) {
-                        Map<String, String> map = invokeGetParameters(getClass(), this);
-                        map = map == null ? new HashMap<>() : map;
-                        map.putAll(convert(StringUtils.parseParameters(value), ""));
-                        invokeSetParameters(getClass(), this, map);
+                        invokeSetParameters(convert(StringUtils.parseParameters(value), ""));
+                    } else {
+                        // in this case, maybe parameters.item3=value3.
+                        invokeSetParameters(ConfigurationUtils.getSubProperties(subProperties, PARAMETERS));

Review comment:
       Should we do `convert()` parameters or not? 
   For compatibility, convert is needed, but additional parameters will be added, which leads to more url parameters, which is not good. This compatibility does not know what the specific problem is, it is best to deal with it when the parameters are read.
   

##########
File path: dubbo-common/src/main/java/org/apache/dubbo/config/AbstractConfig.java
##########
@@ -559,6 +560,13 @@ public void refresh() {
         postProcessRefresh();
     }
 
+    private void invokeSetParameters(Map<String, String> values) {
+        Map<String, String> map = invokeGetParameters(getClass(), this);

Review comment:
       It is better to check argument `Map<String, String> values`, If it is null or empty, just return




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org