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 <ken.lj...@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() {