From dfd135eb95d92ffc3519426b812e274f43b49e9d Mon Sep 17 00:00:00 2001 From: TomlongTK Date: Wed, 8 May 2024 14:09:08 +0800 Subject: [PATCH] Fix the problem of multiple namespaces in MetadataReport (#13971) * Fix the problem of multiple namespaces in MetadataReport * refine * Remove registry config setting * Add some unit tests * Revert getId * Fix cache key --- .../java/org/apache/dubbo/common/URL.java | 4 + .../apache/dubbo/config/RegistryConfig.java | 2 - .../java/org/apache/dubbo/common/URLTest.java | 18 +++ .../deploy/DefaultApplicationDeployer.java | 2 +- .../config/utils/ConfigValidationUtils.java | 10 ++ .../config/bootstrap/DubboBootstrapTest.java | 125 +++++++++++++++++- .../report/MetadataReportInstance.java | 19 ++- .../AbstractMetadataReportFactory.java | 7 +- .../AbstractMetadataReportFactoryTest.java | 22 +++ .../nacos/NacosMetadataReportFactory.java | 14 -- .../client/AbstractServiceDiscovery.java | 2 +- 11 files changed, 187 insertions(+), 38 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/URL.java b/dubbo-common/src/main/java/org/apache/dubbo/common/URL.java index eaf8ffd401..ca79d6480d 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/URL.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/URL.java @@ -1397,6 +1397,10 @@ public /*final**/ class URL implements Serializable { return buildString(true, false, true, true); } + public String toServiceString(String... parameters) { + return buildString(true, true, true, true, parameters); + } + @Deprecated public String getServiceName() { return getServiceInterface(); diff --git a/dubbo-common/src/main/java/org/apache/dubbo/config/RegistryConfig.java b/dubbo-common/src/main/java/org/apache/dubbo/config/RegistryConfig.java index 294916c8b5..71c047ed38 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/config/RegistryConfig.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/config/RegistryConfig.java @@ -28,7 +28,6 @@ import static org.apache.dubbo.common.constants.CommonConstants.EXTRA_KEYS_KEY; import static org.apache.dubbo.common.constants.CommonConstants.SHUTDOWN_WAIT_KEY; import static org.apache.dubbo.common.constants.RegistryConstants.ENABLE_EMPTY_PROTECTION_KEY; import static org.apache.dubbo.common.constants.RegistryConstants.REGISTER_MODE_KEY; -import static org.apache.dubbo.common.constants.RegistryConstants.REGISTRY_CLUSTER_KEY; import static org.apache.dubbo.common.constants.RemotingConstants.BACKUP_KEY; import static org.apache.dubbo.common.utils.PojoUtils.updatePropertyIfAbsent; @@ -221,7 +220,6 @@ public class RegistryConfig extends AbstractConfig { } @Override - @Parameter(key = REGISTRY_CLUSTER_KEY) public String getId() { return super.getId(); } diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/URLTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/URLTest.java index ee03745d3d..eb8e12fb83 100644 --- a/dubbo-common/src/test/java/org/apache/dubbo/common/URLTest.java +++ b/dubbo-common/src/test/java/org/apache/dubbo/common/URLTest.java @@ -1132,4 +1132,22 @@ class URLTest { assertEquals(20881, url.getPort()); assertEquals("apache", url.getParameter("name")); } + + @Test + void testToServiceString() { + URL url = URL.valueOf( + "zookeeper://10.20.130.230:4444/org.apache.dubbo.metadata.report.MetadataReport?version=1.0.0&application=vic&group=aaa"); + assertEquals( + "zookeeper://10.20.130.230:4444/aaa/org.apache.dubbo.metadata.report.MetadataReport:1.0.0", + url.toServiceString()); + } + + @Test + void testToServiceStringWithParameters() { + URL url = URL.valueOf( + "zookeeper://10.20.130.230:4444/org.apache.dubbo.metadata.report.MetadataReport?version=1.0.0&application=vic&group=aaa&namespace=test"); + assertEquals( + "zookeeper://10.20.130.230:4444/aaa/org.apache.dubbo.metadata.report.MetadataReport:1.0.0?namespace=test", + url.toServiceString("namespace")); + } } diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java index 3051827ef5..f58c9c2b24 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java @@ -329,7 +329,7 @@ public class DefaultApplicationDeployer extends AbstractDeployer urls = UrlUtils.parseURLs(address, map); for (URL url : urls) { diff --git a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/bootstrap/DubboBootstrapTest.java b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/bootstrap/DubboBootstrapTest.java index df8a81224c..c419375ebe 100644 --- a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/bootstrap/DubboBootstrapTest.java +++ b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/bootstrap/DubboBootstrapTest.java @@ -37,6 +37,8 @@ import org.apache.dubbo.config.metadata.ExporterDeployListener; import org.apache.dubbo.config.provider.impl.DemoServiceImpl; import org.apache.dubbo.config.utils.ConfigValidationUtils; import org.apache.dubbo.metadata.MetadataService; +import org.apache.dubbo.metadata.report.MetadataReport; +import org.apache.dubbo.metadata.report.MetadataReportInstance; import org.apache.dubbo.monitor.MonitorService; import org.apache.dubbo.registry.RegistryService; import org.apache.dubbo.rpc.Exporter; @@ -50,10 +52,14 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.stream.Collectors; +import com.google.common.collect.Maps; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -61,10 +67,15 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import static org.apache.dubbo.common.constants.CommonConstants.CONFIG_NAMESPACE_KEY; +import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY; import static org.apache.dubbo.common.constants.CommonConstants.DUBBO_MONITOR_ADDRESS; import static org.apache.dubbo.common.constants.CommonConstants.REMOTE_METADATA_STORAGE_TYPE; import static org.apache.dubbo.common.constants.CommonConstants.SHUTDOWN_WAIT_KEY; import static org.apache.dubbo.common.constants.CommonConstants.SHUTDOWN_WAIT_SECONDS_KEY; +import static org.apache.dubbo.common.constants.RegistryConstants.REGISTRY_CLUSTER_KEY; +import static org.apache.dubbo.common.constants.RegistryConstants.REGISTRY_KEY; +import static org.apache.dubbo.metadata.MetadataConstants.REPORT_CONSUMER_URL_KEY; import static org.hamcrest.CoreMatchers.anything; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasEntry; @@ -139,13 +150,29 @@ class DubboBootstrapTest { @Test void testLoadRegistries() { - SysProps.setProperty("dubbo.registry.address", "addr1"); - ServiceConfig serviceConfig = new ServiceConfig(); serviceConfig.setInterface(DemoService.class); serviceConfig.setRef(new DemoServiceImpl()); serviceConfig.setApplication(new ApplicationConfig("testLoadRegistries")); + String registryId = "nacosRegistry"; + String namespace1 = "test"; + RegistryConfig registryConfig = new RegistryConfig(); + registryConfig.setId(registryId); + registryConfig.setAddress("nacos://addr1:8848"); + Map registryParamMap = Maps.newHashMap(); + registryParamMap.put(CONFIG_NAMESPACE_KEY, namespace1); + registryConfig.setParameters(registryParamMap); + + String namespace2 = "test2"; + RegistryConfig registryConfig2 = new RegistryConfig(); + registryConfig2.setAddress("polaris://addr1:9999"); + Map registryParamMap2 = Maps.newHashMap(); + registryParamMap2.put(CONFIG_NAMESPACE_KEY, namespace2); + registryConfig2.setParameters(registryParamMap2); + + serviceConfig.setRegistries(Arrays.asList(registryConfig, registryConfig2)); + // load configs from props DubboBootstrap.getInstance().initialize(); @@ -154,16 +181,104 @@ class DubboBootstrapTest { // ApplicationModel.defaultModel().getEnvironment().setDynamicConfiguration(new // CompositeDynamicConfiguration()); List urls = ConfigValidationUtils.loadRegistries(serviceConfig, true); - Assertions.assertEquals(2, urls.size()); - for (URL url : urls) { + Assertions.assertEquals(4, urls.size()); + + Map> urlsMap = + urls.stream().collect(Collectors.groupingBy(url -> url.getParameter(REGISTRY_KEY))); + Assertions.assertEquals(2, urlsMap.get("nacos").size()); + for (URL url : urlsMap.get("nacos")) { Assertions.assertTrue(url.getProtocol().contains("registry")); - Assertions.assertEquals("addr1:9090", url.getAddress()); + Assertions.assertEquals("addr1:8848", url.getAddress()); Assertions.assertEquals(RegistryService.class.getName(), url.getPath()); + Assertions.assertEquals(registryId + ":" + namespace1, url.getParameter(REGISTRY_CLUSTER_KEY)); Assertions.assertTrue(url.getParameters().containsKey("timestamp")); Assertions.assertTrue(url.getParameters().containsKey("pid")); Assertions.assertTrue(url.getParameters().containsKey("registry")); Assertions.assertTrue(url.getParameters().containsKey("dubbo")); } + + Assertions.assertEquals(2, urlsMap.get("polaris").size()); + for (URL url : urlsMap.get("polaris")) { + Assertions.assertTrue(url.getProtocol().contains("registry")); + Assertions.assertEquals("addr1:9999", url.getAddress()); + Assertions.assertEquals(RegistryService.class.getName(), url.getPath()); + Assertions.assertEquals(DEFAULT_KEY + ":" + namespace2, url.getParameter(REGISTRY_CLUSTER_KEY)); + Assertions.assertTrue(url.getParameters().containsKey("timestamp")); + Assertions.assertTrue(url.getParameters().containsKey("pid")); + Assertions.assertTrue(url.getParameters().containsKey("registry")); + Assertions.assertTrue(url.getParameters().containsKey("dubbo")); + } + } + + @Test + void testRegistryWithMetadataReport() { + ServiceConfig serviceConfig = new ServiceConfig(); + serviceConfig.setInterface(DemoService.class); + serviceConfig.setRef(new DemoServiceImpl()); + + List registryConfigs = new ArrayList<>(); + List metadataReportConfigs = new ArrayList<>(); + + String registryId = "nacosRegistry"; + String namespace1 = "test"; + RegistryConfig registryConfig = new RegistryConfig(); + registryConfig.setId(registryId); + registryConfig.setAddress(zkServerAddress); + Map registryParamMap = Maps.newHashMap(); + registryParamMap.put(CONFIG_NAMESPACE_KEY, namespace1); + registryConfig.setParameters(registryParamMap); + registryConfigs.add(registryConfig); + + MetadataReportConfig metadataReportConfig = new MetadataReportConfig(); + metadataReportConfig.setRegistry(registryId); + metadataReportConfig.setAddress(registryConfig.getAddress()); + Map metadataParamMap = Maps.newHashMap(); + metadataParamMap.put(CONFIG_NAMESPACE_KEY, namespace1); + metadataParamMap.put(REPORT_CONSUMER_URL_KEY, Boolean.TRUE.toString()); + metadataReportConfig.setParameters(metadataParamMap); + metadataReportConfig.setReportMetadata(true); + metadataReportConfigs.add(metadataReportConfig); + + String namespace2 = "test2"; + RegistryConfig registryConfig2 = new RegistryConfig(); + registryConfig2.setAddress(zkServerAddress); + Map registryParamMap2 = Maps.newHashMap(); + registryParamMap2.put(CONFIG_NAMESPACE_KEY, namespace2); + registryConfig2.setParameters(registryParamMap2); + registryConfigs.add(registryConfig2); + + MetadataReportConfig metadataReportConfig2 = new MetadataReportConfig(); + metadataReportConfig2.setAddress(registryConfig2.getAddress()); + Map metadataParamMap2 = Maps.newHashMap(); + metadataParamMap2.put(CONFIG_NAMESPACE_KEY, namespace2); + metadataParamMap2.put(REPORT_CONSUMER_URL_KEY, Boolean.TRUE.toString()); + metadataReportConfig2.setParameters(metadataParamMap2); + metadataReportConfig2.setReportMetadata(true); + metadataReportConfigs.add(metadataReportConfig2); + + serviceConfig.setRegistries(registryConfigs); + + DubboBootstrap.getInstance() + .application(new ApplicationConfig("testRegistryWithMetadataReport")) + .registries(registryConfigs) + .metadataReports(metadataReportConfigs) + .service(serviceConfig) + .protocol(new ProtocolConfig(CommonConstants.DUBBO_PROTOCOL, -1)) + .start(); + + ApplicationModel applicationModel = DubboBootstrap.getInstance().getApplicationModel(); + MetadataReportInstance metadataReportInstance = + applicationModel.getBeanFactory().getBean(MetadataReportInstance.class); + + Map metadataReports = metadataReportInstance.getMetadataReports(true); + Assertions.assertEquals(2, metadataReports.size()); + + List urls = ConfigValidationUtils.loadRegistries(serviceConfig, true); + Assertions.assertEquals(4, urls.size()); + + for (URL url : urls) { + Assertions.assertTrue(metadataReports.containsKey(url.getParameter(REGISTRY_CLUSTER_KEY))); + } } @Test diff --git a/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/MetadataReportInstance.java b/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/MetadataReportInstance.java index ded589b471..59d2dcb874 100644 --- a/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/MetadataReportInstance.java +++ b/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/MetadataReportInstance.java @@ -54,7 +54,7 @@ import static org.apache.dubbo.metadata.report.support.Constants.METADATA_REPORT */ public class MetadataReportInstance implements Disposable { - private AtomicBoolean init = new AtomicBoolean(false); + private final AtomicBoolean initialized = new AtomicBoolean(false); private String metadataType; // mapping of registry id to metadata report instance, registry instances will use this mapping to find related @@ -69,7 +69,7 @@ public class MetadataReportInstance implements Disposable { } public void init(List metadataReportConfigs) { - if (!init.compareAndSet(false, true)) { + if (!initialized.compareAndSet(false, true)) { return; } @@ -114,9 +114,10 @@ public class MetadataReportInstance implements Disposable { } private String getRelatedRegistryId(MetadataReportConfig config, URL url) { - String relatedRegistryId = isEmpty(config.getRegistry()) - ? (isEmpty(config.getId()) ? DEFAULT_KEY : config.getId()) - : config.getRegistry(); + String relatedRegistryId = config.getRegistry(); + if (isEmpty(relatedRegistryId)) { + relatedRegistryId = DEFAULT_KEY; + } String namespace = url.getParameter(NAMESPACE_KEY); if (!StringUtils.isEmpty(namespace)) { relatedRegistryId += ":" + namespace; @@ -144,15 +145,13 @@ public class MetadataReportInstance implements Disposable { return metadataType; } - public boolean inited() { - return init.get(); + public boolean isInitialized() { + return initialized.get(); } @Override public void destroy() { - metadataReports.forEach((_k, reporter) -> { - reporter.destroy(); - }); + metadataReports.forEach((k, reporter) -> reporter.destroy()); metadataReports.clear(); } } diff --git a/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/support/AbstractMetadataReportFactory.java b/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/support/AbstractMetadataReportFactory.java index 7f1770291f..ecc34f1047 100644 --- a/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/support/AbstractMetadataReportFactory.java +++ b/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/support/AbstractMetadataReportFactory.java @@ -29,6 +29,7 @@ import java.util.concurrent.locks.ReentrantLock; import static org.apache.dubbo.common.constants.CommonConstants.CHECK_KEY; import static org.apache.dubbo.common.constants.LoggerCodeConstants.COMMON_UNEXPECTED_EXCEPTION; import static org.apache.dubbo.common.constants.LoggerCodeConstants.PROXY_FAILED_EXPORT_SERVICE; +import static org.apache.dubbo.metadata.MetadataConstants.NAMESPACE_KEY; public abstract class AbstractMetadataReportFactory implements MetadataReportFactory { @@ -50,7 +51,7 @@ public abstract class AbstractMetadataReportFactory implements MetadataReportFac @Override public MetadataReport getMetadataReport(URL url) { url = url.setPath(MetadataReport.class.getName()).removeParameters(EXPORT_KEY, REFER_KEY); - String key = toMetadataReportKey(url); + String key = url.toServiceString(NAMESPACE_KEY); MetadataReport metadataReport = serviceStoreMap.get(key); if (metadataReport != null) { @@ -88,10 +89,6 @@ public abstract class AbstractMetadataReportFactory implements MetadataReportFac } } - protected String toMetadataReportKey(URL url) { - return url.toServiceString(); - } - @Override public void destroy() { lock.lock(); diff --git a/dubbo-metadata/dubbo-metadata-api/src/test/java/org/apache/dubbo/metadata/report/support/AbstractMetadataReportFactoryTest.java b/dubbo-metadata/dubbo-metadata-api/src/test/java/org/apache/dubbo/metadata/report/support/AbstractMetadataReportFactoryTest.java index f8c3dae0d9..d321a4ba92 100644 --- a/dubbo-metadata/dubbo-metadata-api/src/test/java/org/apache/dubbo/metadata/report/support/AbstractMetadataReportFactoryTest.java +++ b/dubbo-metadata/dubbo-metadata-api/src/test/java/org/apache/dubbo/metadata/report/support/AbstractMetadataReportFactoryTest.java @@ -145,4 +145,26 @@ class AbstractMetadataReportFactoryTest { MetadataReport metadataReport2 = metadataReportFactory.getMetadataReport(url2); Assertions.assertNotEquals(metadataReport1, metadataReport2); } + + @Test + void testGetForSameNamespace() { + URL url1 = URL.valueOf("zookeeper://" + NetUtils.getLocalAddress().getHostName() + + ":4444/org.apache.dubbo.TestService1?version=1.0.0&application=vic&namespace=test"); + URL url2 = URL.valueOf("zookeeper://" + NetUtils.getLocalAddress().getHostName() + + ":4444/org.apache.dubbo.TestService2?version=1.0.0&application=vic&namespace=test"); + MetadataReport metadataReport1 = metadataReportFactory.getMetadataReport(url1); + MetadataReport metadataReport2 = metadataReportFactory.getMetadataReport(url2); + Assertions.assertEquals(metadataReport1, metadataReport2); + } + + @Test + void testGetForDiffNamespace() { + URL url1 = URL.valueOf("zookeeper://" + NetUtils.getLocalAddress().getHostName() + + ":4444/org.apache.dubbo.TestService?version=1.0.0&application=vic&namespace=test"); + URL url2 = URL.valueOf("zookeeper://" + NetUtils.getLocalAddress().getHostName() + + ":4444/org.apache.dubbo.TestService?version=1.0.0&application=vic&namespace=dev"); + MetadataReport metadataReport1 = metadataReportFactory.getMetadataReport(url1); + MetadataReport metadataReport2 = metadataReportFactory.getMetadataReport(url2); + Assertions.assertNotEquals(metadataReport1, metadataReport2); + } } diff --git a/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReportFactory.java b/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReportFactory.java index 338b4067b7..a858a0a1b1 100644 --- a/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReportFactory.java +++ b/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReportFactory.java @@ -17,12 +17,9 @@ package org.apache.dubbo.metadata.store.nacos; import org.apache.dubbo.common.URL; -import org.apache.dubbo.common.utils.StringUtils; import org.apache.dubbo.metadata.report.MetadataReport; import org.apache.dubbo.metadata.report.support.AbstractMetadataReportFactory; -import static org.apache.dubbo.metadata.MetadataConstants.NAMESPACE_KEY; - /** * metadata report factory impl for nacos */ @@ -31,15 +28,4 @@ public class NacosMetadataReportFactory extends AbstractMetadataReportFactory { protected MetadataReport createMetadataReport(URL url) { return new NacosMetadataReport(url); } - - @Override - protected String toMetadataReportKey(URL url) { - String namespace = url.getParameter(NAMESPACE_KEY); - if (!StringUtils.isEmpty(namespace)) { - return URL.valueOf(url.toServiceString()) - .addParameter(NAMESPACE_KEY, namespace) - .toString(); - } - return super.toMetadataReportKey(url); - } } diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java index 341d8926f8..4877581321 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java @@ -84,7 +84,7 @@ public abstract class AbstractServiceDiscovery implements ServiceDiscovery { this(applicationModel, applicationModel.getApplicationName(), registryURL); MetadataReportInstance metadataReportInstance = applicationModel.getBeanFactory().getBean(MetadataReportInstance.class); - metadataType = metadataReportInstance.getMetadataType(); + this.metadataType = metadataReportInstance.getMetadataType(); this.metadataReport = metadataReportInstance.getMetadataReport(registryURL.getParameter(REGISTRY_CLUSTER_KEY)); }