You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by vi...@apache.org on 2019/07/12 02:06:06 UTC
[dubbo] branch 2.7.3-release updated: [Dubbo-4525] fix
Clusterutils.mergeurl,
make sure specific keys from provider side not override by that from
consumer. (#4533)
This is an automated email from the ASF dual-hosted git repository.
victory pushed a commit to branch 2.7.3-release
in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/2.7.3-release by this push:
new a5f6090 [Dubbo-4525] fix Clusterutils.mergeurl, make sure specific keys from provider side not override by that from consumer. (#4533)
a5f6090 is described below
commit a5f60906665c2a7bf1ab4d9e2a18257bc17a0545
Author: ken.lj <ke...@gmail.com>
AuthorDate: Fri Jul 12 10:06:01 2019 +0800
[Dubbo-4525] fix Clusterutils.mergeurl, make sure specific keys from provider side not override by that from consumer. (#4533)
Fixes #4525
---
.../dubbo/rpc/cluster/support/ClusterUtils.java | 53 +++++-----------
.../rpc/cluster/support/ClusterUtilsTest.java | 74 +++++++++++++++++++++-
.../ReferenceAnnotationBeanPostProcessorTest.java | 6 +-
.../annotation/consumer/ConsumerConfiguration.java | 10 +--
.../consumer/test/TestConsumerConfiguration.java | 10 +--
5 files changed, 104 insertions(+), 49 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
index 7a2894b..17ae72d 100644
--- 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,11 @@
package org.apache.dubbo.rpc.cluster.support;
import org.apache.dubbo.common.URL;
-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 +39,7 @@ import static org.apache.dubbo.common.constants.CommonConstants.VERSION_KEY;
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,55 +80,35 @@ public class ClusterUtils {
}
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);
- }
- }
- 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<String, String> copyOfLocalMap = new HashMap<>(localMap);
+ copyOfLocalMap.remove(GROUP_KEY);
+ copyOfLocalMap.remove(RELEASE_KEY);
+ copyOfLocalMap.remove(DUBBO_VERSION_KEY);
+ copyOfLocalMap.remove(VERSION_KEY);
+ copyOfLocalMap.remove(METHODS_KEY);
+ copyOfLocalMap.remove(TIMESTAMP_KEY);
+ copyOfLocalMap.remove(TAG_KEY);
+
+ map.putAll(copyOfLocalMap);
+
map.put(REMOTE_APPLICATION_KEY, remoteMap.get(APPLICATION_KEY));
// Combine filters and listeners on Provider and Consumer
String remoteFilter = remoteMap.get(REFERENCE_FILTER_KEY);
- String localFilter = localMap.get(REFERENCE_FILTER_KEY);
+ String localFilter = copyOfLocalMap.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);
+ String localListener = copyOfLocalMap.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) {
- String remoteValue = remoteMap.get(key);
- if (StringUtils.isNotEmpty(remoteValue)) {
- map.put(key, remoteValue);
- }
- }
-
}
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
index 871e075..1c2b520 100644
--- 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.Assertions;
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 class ClusterUtilsTest {
.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 class ClusterUtilsTest {
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×tamp=local");
+ URL remoteURL = URL.valueOf("dubbo://localhost:20880/DemoService?version=remote&group=remote&dubbo=remote&release=remote" +
+ "&methods=remote&tag=remote×tamp=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×tamp=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×tamp=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/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java
index b5f5ef0..56fd297 100644
--- a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java
+++ b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java
@@ -54,7 +54,7 @@ import static org.junit.Assert.assertTrue;
@TestPropertySource(properties = {
"packagesToScan = org.apache.dubbo.config.spring.context.annotation.provider",
"consumer.version = ${demo.service.version}",
- "consumer.url = dubbo://127.0.0.1:12345",
+ "consumer.url = dubbo://127.0.0.1:12345?version=2.5.7",
})
public class ReferenceAnnotationBeanPostProcessorTest {
@@ -221,7 +221,7 @@ public class ReferenceAnnotationBeanPostProcessorTest {
return demoServiceFromAncestor;
}
- @Reference(id = "my-reference-bean", version = "2.5.7", url = "dubbo://127.0.0.1:12345")
+ @Reference(id = "my-reference-bean", version = "2.5.7", url = "dubbo://127.0.0.1:12345?version=2.5.7")
public void setDemoServiceFromAncestor(DemoService demoServiceFromAncestor) {
this.demoServiceFromAncestor = demoServiceFromAncestor;
}
@@ -259,7 +259,7 @@ public class ReferenceAnnotationBeanPostProcessorTest {
return demoService;
}
- @com.alibaba.dubbo.config.annotation.Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345")
+ @com.alibaba.dubbo.config.annotation.Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345?version=2.5.7")
public void setDemoService(DemoService demoService) {
this.demoService = demoService;
}
diff --git a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/context/annotation/consumer/ConsumerConfiguration.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/context/annotation/consumer/ConsumerConfiguration.java
index 47c90e4..a9109f9 100644
--- a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/context/annotation/consumer/ConsumerConfiguration.java
+++ b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/context/annotation/consumer/ConsumerConfiguration.java
@@ -34,6 +34,8 @@ import org.springframework.context.annotation.PropertySource;
@PropertySource("META-INF/default.properties")
public class ConsumerConfiguration {
+ private static final String remoteURL = "dubbo://127.0.0.1:12345?version=2.5.7";
+
/**
* Current application configuration, to replace XML config:
* <prev>
@@ -67,7 +69,7 @@ public class ConsumerConfiguration {
@Autowired
private DemoService autowiredDemoService;
- @Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345")
+ @Reference(version = "2.5.7", url = remoteURL)
private DemoService demoService;
public DemoService getDemoService() {
@@ -86,7 +88,7 @@ public class ConsumerConfiguration {
public static abstract class Ancestor {
- @Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345")
+ @Reference(version = "2.5.7", url = remoteURL)
private DemoService demoServiceFromAncestor;
public DemoService getDemoServiceFromAncestor() {
@@ -106,7 +108,7 @@ public class ConsumerConfiguration {
return demoServiceFromParent;
}
- @Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345")
+ @Reference(version = "2.5.7", url = remoteURL)
public void setDemoServiceFromParent(DemoService demoServiceFromParent) {
this.demoServiceFromParent = demoServiceFromParent;
}
@@ -118,7 +120,7 @@ public class ConsumerConfiguration {
@Autowired
private DemoService demoService;
- @Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345")
+ @Reference(version = "2.5.7", url = remoteURL)
private DemoService demoServiceFromChild;
diff --git a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/context/annotation/consumer/test/TestConsumerConfiguration.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/context/annotation/consumer/test/TestConsumerConfiguration.java
index 82960ef..c924390 100644
--- a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/context/annotation/consumer/test/TestConsumerConfiguration.java
+++ b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/context/annotation/consumer/test/TestConsumerConfiguration.java
@@ -35,7 +35,9 @@ import org.springframework.transaction.annotation.EnableTransactionManagement;
@EnableTransactionManagement
public class TestConsumerConfiguration {
- @Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345", application = "dubbo-demo-application")
+ private static final String remoteURL = "dubbo://127.0.0.1:12345?version=2.5.7";
+
+ @Reference(version = "2.5.7", url = remoteURL, application = "dubbo-demo-application")
private DemoService demoService;
@Autowired
@@ -61,7 +63,7 @@ public class TestConsumerConfiguration {
public static abstract class Ancestor {
- @Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345", application = "dubbo-demo-application")
+ @Reference(version = "2.5.7", url = remoteURL, application = "dubbo-demo-application")
private DemoService demoServiceFromAncestor;
public DemoService getDemoServiceFromAncestor() {
@@ -81,7 +83,7 @@ public class TestConsumerConfiguration {
return demoServiceFromParent;
}
- @Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345", application = "dubbo-demo-application")
+ @Reference(version = "2.5.7", url = remoteURL, application = "dubbo-demo-application")
public void setDemoServiceFromParent(DemoService demoServiceFromParent) {
this.demoServiceFromParent = demoServiceFromParent;
}
@@ -90,7 +92,7 @@ public class TestConsumerConfiguration {
public static class Child extends TestConsumerConfiguration.Parent {
- @Reference(version = "2.5.7", url = "dubbo://127.0.0.1:12345", application = "dubbo-demo-application")
+ @Reference(version = "2.5.7", url = remoteURL, application = "dubbo-demo-application")
private DemoService demoServiceFromChild;
public DemoService getDemoServiceFromChild() {