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

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();

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();
     }

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"));
+    }
 }

dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java

@@ -329,7 +329,7 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
             }
         }
         metadataReportInstance.init(validMetadataReportConfigs);
-        if (!metadataReportInstance.inited()) {
+        if (!metadataReportInstance.isInitialized()) {
             throw new IllegalStateException(String.format(
                     "%s MetadataConfigs found, but none of them is valid.", metadataReportConfigs.size()));
         }

dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/utils/ConfigValidationUtils.java

@@ -82,6 +82,7 @@ import java.util.stream.Collectors;
 
 import static org.apache.dubbo.common.constants.CommonConstants.ANYHOST_VALUE;
 import static org.apache.dubbo.common.constants.CommonConstants.CLUSTER_KEY;
+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.DUBBO_PROTOCOL;
@@ -108,6 +109,7 @@ import static org.apache.dubbo.common.constants.RegistryConstants.DEFAULT_REGIST
 import static org.apache.dubbo.common.constants.RegistryConstants.DEFAULT_REGISTER_MODE_INTERFACE;
 import static org.apache.dubbo.common.constants.RegistryConstants.DUBBO_REGISTER_MODE_DEFAULT_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.RegistryConstants.REGISTRY_KEY;
 import static org.apache.dubbo.common.constants.RegistryConstants.REGISTRY_PROTOCOL;
 import static org.apache.dubbo.common.constants.RegistryConstants.REGISTRY_TYPE_KEY;
@@ -217,6 +219,14 @@ public class ConfigValidationUtils {
                     if (!map.containsKey(PROTOCOL_KEY)) {
                         map.put(PROTOCOL_KEY, DUBBO_PROTOCOL);
                     }
+                    String registryCluster = config.getId();
+                    if (isEmpty(registryCluster)) {
+                        registryCluster = DEFAULT_KEY;
+                    }
+                    if (map.containsKey(CONFIG_NAMESPACE_KEY)) {
+                        registryCluster += ":" + map.get(CONFIG_NAMESPACE_KEY);
+                    }
+                    map.put(REGISTRY_CLUSTER_KEY, registryCluster);
                     List<URL> urls = UrlUtils.parseURLs(address, map);
 
                     for (URL url : urls) {

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<String, String> 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<String, String> 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<URL> urls = ConfigValidationUtils.loadRegistries(serviceConfig, true);
-        Assertions.assertEquals(2, urls.size());
+        Assertions.assertEquals(4, urls.size());
-        for (URL url : urls) {
+
+        Map<String, List<URL>> 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<RegistryConfig> registryConfigs = new ArrayList<>();
+        List<MetadataReportConfig> metadataReportConfigs = new ArrayList<>();
+
+        String registryId = "nacosRegistry";
+        String namespace1 = "test";
+        RegistryConfig registryConfig = new RegistryConfig();
+        registryConfig.setId(registryId);
+        registryConfig.setAddress(zkServerAddress);
+        Map<String, String> 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<String, String> 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<String, String> registryParamMap2 = Maps.newHashMap();
+        registryParamMap2.put(CONFIG_NAMESPACE_KEY, namespace2);
+        registryConfig2.setParameters(registryParamMap2);
+        registryConfigs.add(registryConfig2);
+
+        MetadataReportConfig metadataReportConfig2 = new MetadataReportConfig();
+        metadataReportConfig2.setAddress(registryConfig2.getAddress());
+        Map<String, String> 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<String, MetadataReport> metadataReports = metadataReportInstance.getMetadataReports(true);
+        Assertions.assertEquals(2, metadataReports.size());
+
+        List<URL> urls = ConfigValidationUtils.loadRegistries(serviceConfig, true);
+        Assertions.assertEquals(4, urls.size());
+
+        for (URL url : urls) {
+            Assertions.assertTrue(metadataReports.containsKey(url.getParameter(REGISTRY_CLUSTER_KEY)));
+        }
     }
 
     @Test

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<MetadataReportConfig> 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())
+        String relatedRegistryId = config.getRegistry();
-                ? (isEmpty(config.getId()) ? DEFAULT_KEY : config.getId())
+        if (isEmpty(relatedRegistryId)) {
-                : config.getRegistry();
+            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() {
+    public boolean isInitialized() {
-        return init.get();
+        return initialized.get();
     }
 
     @Override
     public void destroy() {
-        metadataReports.forEach((_k, reporter) -> {
+        metadataReports.forEach((k, reporter) -> reporter.destroy());
-            reporter.destroy();
-        });
         metadataReports.clear();
     }
 }

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();

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);
+    }
 }

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);
-    }
 }

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));
     }