url merge

chickenlj · Jul 11, 2019 · 79ae133 · 79ae133
1 parent 19d0e44
commit 79ae133
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 27 deletions.
diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/ClusterUtils.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/ClusterUtils.java
@@ -17,13 +17,13 @@
 package org.apache.dubbo.rpc.cluster.support;
 
 import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.utils.CollectionUtils;
 import org.apache.dubbo.common.utils.StringUtils;
 import org.apache.dubbo.remoting.Constants;
 
 import java.util.HashMap;
 import java.util.Map;
 
-import static org.apache.dubbo.rpc.cluster.Constants.TAG_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.ALIVE_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.APPLICATION_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.CORE_THREADS_KEY;
@@ -41,6 +41,7 @@
 import static org.apache.dubbo.remoting.Constants.DUBBO_VERSION_KEY;
 import static org.apache.dubbo.rpc.Constants.INVOKER_LISTENER_KEY;
 import static org.apache.dubbo.rpc.Constants.REFERENCE_FILTER_KEY;
+import static org.apache.dubbo.rpc.cluster.Constants.TAG_KEY;
 
 /**
  * ClusterUtils
@@ -81,29 +82,23 @@ public static URL mergeUrl(URL remoteUrl, Map<String, String> localMap) {
         }
 
         if (localMap != null && localMap.size() > 0) {
-            // All providers come to here have been filtered by group, which means only those providers that have the exact same group value with the consumer could come to here.
-            // So, generally, we don't need to care about the group value here.
-            // But when comes to group merger, there is an exception, the consumer group may be '*' while the provider group can be empty or any other values.
-            String remoteGroup = map.get(GROUP_KEY);
-            String remoteRelease = map.get(RELEASE_KEY);
             map.putAll(localMap);
-            if (StringUtils.isNotEmpty(remoteGroup)) {
-                map.put(GROUP_KEY, remoteGroup);
-            }
-            // we should always keep the Provider RELEASE_KEY not overrode by the the value on Consumer side.
-            map.remove(RELEASE_KEY);
-            if (StringUtils.isNotEmpty(remoteRelease)) {
-                map.put(RELEASE_KEY, remoteRelease);
-            }
         }
+
+        // All providers reached here have been filtered by group, which means only those providers that have the exact
+        // same group value with the consumer could reach here. Based on this we don't need to care about the group value here.
+        // But when comes to group merger, there is an exception, the consumer group may use '*' while the provider
+        // group can be empty or any other values.
+        reserveRemoteValue(GROUP_KEY, map, remoteMap);
+        reserveRemoteValue(RELEASE_KEY, map, remoteMap);
+        // Use version passed from provider side
+        reserveRemoteValue(DUBBO_VERSION_KEY, map, remoteMap);
+        reserveRemoteValue(VERSION_KEY, map, remoteMap);
+        reserveRemoteValue(METHODS_KEY, map, remoteMap);
+        reserveRemoteValue(TIMESTAMP_KEY, map, remoteMap);
+        reserveRemoteValue(TAG_KEY, map, remoteMap);
+
         if (remoteMap != null && remoteMap.size() > 0) {
-            // Use version passed from provider side
-            reserveRemoteValue(DUBBO_VERSION_KEY, map, remoteMap);
-            reserveRemoteValue(VERSION_KEY, map, remoteMap);
-            reserveRemoteValue(METHODS_KEY, map, remoteMap);
-            reserveRemoteValue(TIMESTAMP_KEY, map, remoteMap);
-            reserveRemoteValue(TAG_KEY, map, remoteMap);
-            // TODO, for compatibility consideration, we cannot simply change the value behind APPLICATION_KEY from Consumer to Provider. So just add an extra key here.
             // Reserve application name from provider.
             map.put(REMOTE_APPLICATION_KEY, remoteMap.get(APPLICATION_KEY));
 
@@ -112,23 +107,29 @@ public static URL mergeUrl(URL remoteUrl, Map<String, String> localMap) {
             String localFilter = localMap.get(REFERENCE_FILTER_KEY);
             if (remoteFilter != null && remoteFilter.length() > 0
                     && localFilter != null && localFilter.length() > 0) {
-                localMap.put(REFERENCE_FILTER_KEY, remoteFilter + "," + localFilter);
+                map.put(REFERENCE_FILTER_KEY, remoteFilter + "," + localFilter);
             }
             String remoteListener = remoteMap.get(INVOKER_LISTENER_KEY);
             String localListener = localMap.get(INVOKER_LISTENER_KEY);
             if (remoteListener != null && remoteListener.length() > 0
                     && localListener != null && localListener.length() > 0) {
-                localMap.put(INVOKER_LISTENER_KEY, remoteListener + "," + localListener);
+                map.put(INVOKER_LISTENER_KEY, remoteListener + "," + localListener);
             }
         }
 
         return remoteUrl.clearParameters().addParameters(map);
     }
 
     private static void reserveRemoteValue(String key, Map<String, String> map, Map<String, String> remoteMap) {
+        if (CollectionUtils.isEmptyMap(remoteMap)) {
+            map.remove(key);
+        }
+
         String remoteValue = remoteMap.get(key);
         if (StringUtils.isNotEmpty(remoteValue)) {
             map.put(key, remoteValue);
+        } else {
+            map.remove(key);
         }
     }
 

diff --git a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/ClusterUtilsTest.java b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/ClusterUtilsTest.java
@@ -23,17 +23,27 @@
 import org.junit.jupiter.api.Test;
 
 import static org.apache.dubbo.common.constants.CommonConstants.ALIVE_KEY;
+import static org.apache.dubbo.common.constants.CommonConstants.APPLICATION_KEY;
+import static org.apache.dubbo.common.constants.CommonConstants.CLUSTER_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.CORE_THREADS_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY_PREFIX;
+import static org.apache.dubbo.common.constants.CommonConstants.DUBBO_PROTOCOL;
 import static org.apache.dubbo.common.constants.CommonConstants.GROUP_KEY;
+import static org.apache.dubbo.common.constants.CommonConstants.METHODS_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.PID_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.QUEUES_KEY;
+import static org.apache.dubbo.common.constants.CommonConstants.RELEASE_KEY;
+import static org.apache.dubbo.common.constants.CommonConstants.REMOTE_APPLICATION_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.THREADPOOL_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.THREADS_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.THREAD_NAME_KEY;
+import static org.apache.dubbo.common.constants.CommonConstants.TIMEOUT_KEY;
+import static org.apache.dubbo.common.constants.CommonConstants.TIMESTAMP_KEY;
 import static org.apache.dubbo.common.constants.CommonConstants.VERSION_KEY;
-import static org.apache.dubbo.common.constants.CommonConstants.DUBBO_PROTOCOL;
 import static org.apache.dubbo.remoting.Constants.DUBBO_VERSION_KEY;
+import static org.apache.dubbo.rpc.Constants.REFERENCE_FILTER_KEY;
+import static org.apache.dubbo.rpc.cluster.Constants.LOADBALANCE_KEY;
+import static org.apache.dubbo.rpc.cluster.Constants.TAG_KEY;
 
 public class ClusterUtilsTest {
 
@@ -60,11 +70,15 @@ public void testMergeUrl() throws Exception {
                 .addParameter(DEFAULT_KEY_PREFIX + QUEUES_KEY, Integer.MAX_VALUE)
                 .addParameter(DEFAULT_KEY_PREFIX + ALIVE_KEY, Integer.MAX_VALUE)
                 .addParameter(DEFAULT_KEY_PREFIX + THREAD_NAME_KEY, "test")
+                .addParameter(APPLICATION_KEY, "provider")
+                .addParameter(REFERENCE_FILTER_KEY, "filter1,filter2")
                 .build();
 
         URL consumerURL = new URLBuilder(DUBBO_PROTOCOL, "localhost", 55555)
                 .addParameter(PID_KEY, "1234")
                 .addParameter(THREADPOOL_KEY, "foo")
+                .addParameter(APPLICATION_KEY, "consumer")
+                .addParameter(REFERENCE_FILTER_KEY, "filter3")
                 .build();
 
         URL url = ClusterUtils.mergeUrl(providerURL, consumerURL.getParameters());
@@ -91,6 +105,64 @@ public void testMergeUrl() throws Exception {
         Assertions.assertEquals(url.getPassword(), "password");
         Assertions.assertEquals(url.getParameter(PID_KEY), "1234");
         Assertions.assertEquals(url.getParameter(THREADPOOL_KEY), "foo");
+        Assertions.assertEquals(url.getParameter(APPLICATION_KEY), "consumer");
+        Assertions.assertEquals(url.getParameter(REMOTE_APPLICATION_KEY), "provider");
+        Assertions.assertEquals(url.getParameter(REFERENCE_FILTER_KEY), "filter1,filter2,filter3");
+    }
+
+    @Test
+    public void testUseProviderParams() {
+        // present in both local and remote, but uses remote value.
+        URL localURL = URL.valueOf("dubbo://localhost:20880/DemoService?version=local&group=local&dubbo=local&release=local" +
+                "&methods=local&tag=local&timestamp=local");
+        URL remoteURL = URL.valueOf("dubbo://localhost:20880/DemoService?version=remote&group=remote&dubbo=remote&release=remote" +
+                "&methods=remote&tag=remote&timestamp=remote");
+        URL mergedUrl = ClusterUtils.mergeUrl(remoteURL, localURL.getParameters());
+
+        Assertions.assertEquals(remoteURL.getParameter(VERSION_KEY), mergedUrl.getParameter(VERSION_KEY));
+        Assertions.assertEquals(remoteURL.getParameter(GROUP_KEY), mergedUrl.getParameter(GROUP_KEY));
+        Assertions.assertEquals(remoteURL.getParameter(DUBBO_VERSION_KEY), mergedUrl.getParameter(DUBBO_VERSION_KEY));
+        Assertions.assertEquals(remoteURL.getParameter(RELEASE_KEY), mergedUrl.getParameter(RELEASE_KEY));
+        Assertions.assertEquals(remoteURL.getParameter(METHODS_KEY), mergedUrl.getParameter(METHODS_KEY));
+        Assertions.assertEquals(remoteURL.getParameter(TIMESTAMP_KEY), mergedUrl.getParameter(TIMESTAMP_KEY));
+        Assertions.assertEquals(remoteURL.getParameter(TAG_KEY), mergedUrl.getParameter(TAG_KEY));
+
+        // present in local url but not in remote url, parameters of remote url is empty
+        localURL = URL.valueOf("dubbo://localhost:20880/DemoService?version=local&group=local&dubbo=local&release=local" +
+                "&methods=local&tag=local&timestamp=local");
+        remoteURL = URL.valueOf("dubbo://localhost:20880/DemoService");
+        mergedUrl = ClusterUtils.mergeUrl(remoteURL, localURL.getParameters());
+
+        Assertions.assertNull(mergedUrl.getParameter(VERSION_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(GROUP_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(DUBBO_VERSION_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(RELEASE_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(METHODS_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(TIMESTAMP_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(TAG_KEY));
+
+        // present in local url but not in remote url
+        localURL = URL.valueOf("dubbo://localhost:20880/DemoService?version=local&group=local&dubbo=local&release=local" +
+                "&methods=local&tag=local&timestamp=local");
+        remoteURL = URL.valueOf("dubbo://localhost:20880/DemoService?key=value");
+        mergedUrl = ClusterUtils.mergeUrl(remoteURL, localURL.getParameters());
+
+        Assertions.assertNull(mergedUrl.getParameter(VERSION_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(GROUP_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(DUBBO_VERSION_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(RELEASE_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(METHODS_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(TIMESTAMP_KEY));
+        Assertions.assertNull(mergedUrl.getParameter(TAG_KEY));
+
+        // present in both local and remote, uses local url params
+        localURL = URL.valueOf("dubbo://localhost:20880/DemoService?loadbalance=local&timeout=1000&cluster=local");
+        remoteURL = URL.valueOf("dubbo://localhost:20880/DemoService?loadbalance=remote&timeout=2000&cluster=remote");
+        mergedUrl = ClusterUtils.mergeUrl(remoteURL, localURL.getParameters());
+
+        Assertions.assertEquals(localURL.getParameter(CLUSTER_KEY), mergedUrl.getParameter(CLUSTER_KEY));
+        Assertions.assertEquals(localURL.getParameter(TIMEOUT_KEY), mergedUrl.getParameter(TIMEOUT_KEY));
+        Assertions.assertEquals(localURL.getParameter(LOADBALANCE_KEY), mergedUrl.getParameter(LOADBALANCE_KEY));
     }
 
 }
diff --git a/...-demo/dubbo-demo-xml/dubbo-demo-xml-consumer/src/main/resources/spring/dubbo-consumer.xml b/...-demo/dubbo-demo-xml/dubbo-demo-xml-consumer/src/main/resources/spring/dubbo-consumer.xml
@@ -25,6 +25,8 @@
 
     <dubbo:registry address="multicast://224.5.6.7:1234"/>
 
+    <dubbo:consumer tag="tag1" />
+
     <!-- generate proxy for the remote service, then demoService can be used in the same way as the
     local regular interface -->
     <dubbo:reference id="demoService" check="false" interface="org.apache.dubbo.demo.DemoService"/>

diff --git a/dubbo-dependencies-bom/pom.xml b/dubbo-dependencies-bom/pom.xml
@@ -153,7 +153,7 @@
         <mortbay_jetty_version>6.1.26</mortbay_jetty_version>
         <portlet_version>2.0</portlet_version>
         <maven_flatten_version>1.1.0</maven_flatten_version>
-        <revision>2.7.3</revision>
+        <revision>2.7.3-SNAPSHOT</revision>
     </properties>
 
     <dependencyManagement>

diff --git a/dubbo-dependencies/dubbo-dependencies-zookeeper/pom.xml b/dubbo-dependencies/dubbo-dependencies-zookeeper/pom.xml
@@ -32,7 +32,7 @@
     <packaging>pom</packaging>
 
     <properties>
-        <revision>2.7.3</revision>
+        <revision>2.7.3-SNAPSHOT</revision>
         <maven_flatten_version>1.1.0</maven_flatten_version>
     </properties>
 

diff --git a/pom.xml b/pom.xml
@@ -125,7 +125,7 @@
         <arguments />
         <checkstyle.skip>true</checkstyle.skip>
         <rat.skip>true</rat.skip>
-        <revision>2.7.3</revision>
+        <revision>2.7.3-SNAPSHOT</revision>
     </properties>
 
     <modules>