Feature/nacos config

[Moriadry-zz]

What is the purpose of the change

nacos config-server support

Brief changelog

nacos config-server support

Verifying this change

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

• Make sure there is a GITHUB_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
• Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
• Run mvn clean install -DskipTests=false & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
• If this contribution is large, please follow the Software Donation Guide.

[codecov-io]

Codecov Report

Merging #3988 into master will decrease coverage by 0.4%.
The diff coverage is 0.95%.

@@             Coverage Diff              @@
##             master    #3988      +/-   ##
============================================
- Coverage     63.81%   63.41%   -0.41%     
- Complexity       71      565     +494     
============================================
  Files           744      750       +6     
  Lines         32209    32415     +206     
  Branches       5127     5145      +18     
============================================
+ Hits          20555    20556       +1     
- Misses         9305     9500     +195     
- Partials       2349     2359      +10

Impacted Files	Coverage Δ	Complexity Δ
...c/main/java/org/apache/dubbo/common/Constants.java	93.33% <ø> (ø)	0 <0> (ø)	⬇️
...enter/support/nacos/NacosDynamicConfiguration.java	0% <0%> (ø)	0 <0> (?)
...upport/nacos/NacosDynamicConfigurationFactory.java	0% <0%> (ø)	0 <0> (?)
...stry/integration/AbstractConfiguratorListener.java	30.76% <100%> (ø)	0 <0> (ø)	⬇️
...ava/org/apache/dubbo/rpc/filter/GenericFilter.java	50% <0%> (-18.43%)	0% <0%> (ø)
...ache/dubbo/remoting/p2p/support/AbstractGroup.java	45.45% <0%> (-11.37%)	0% <0%> (ø)
.../remoting/transport/netty4/NettyServerHandler.java	61.9% <0%> (-9.53%)	0% <0%> (ø)
.../apache/dubbo/remoting/transport/AbstractPeer.java	63.04% <0%> (-8.7%)	0% <0%> (ø)
...va/org/apache/dubbo/rpc/support/ProtocolUtils.java	72% <0%> (-6.27%)	0% <0%> (ø)
...ng/transport/dispatcher/all/AllChannelHandler.java	51.42% <0%> (-5.72%)	0% <0%> (ø)
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 392fcbf...7a931ef. Read the comment docs.

[ralf0131]

This pull request is targeting #3846 . I will start to review today.

Thanks for the pull request, I was planning to do it. Do you have interest in implementing Nacos as a metadata center? There are many examples in dubbo-metadata-report module. If not, I am planning to do it.

[ralf0131]

I got the following errors when running the testGetConfig unit test for the first time:

java.lang.AssertionError: 
Expected :hello
Actual   :null
 <Click to see difference>

	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at org.junit.Assert.assertEquals(Assert.java:144)
	at org.apache.dubbo.configcenter.support.nacos.NacosDynamicConfigurationTest.testGetConfig(NacosDynamicConfigurationTest.java:47)
       ...

But the second run is OK.

Adding some sleep to between put and getConfig should work fine.

        put("dubbo-config-org.apache.dubbo.nacos.testService.configurators", "hello");
        Thread.sleep(200);
        put("dubbo-config-dubbo.properties:test", "aaa=bbb");
        Thread.sleep(200);
        Assert.assertEquals("hello", config.getConfig("org.apache.dubbo.nacos.testService.configurators"));
        Assert.assertEquals("aaa=bbb", config.getConfig("dubbo.properties", "test"));

[Moriadry-zz]

@ralf0131 I'm fixing this. Also, I feel happy to help with nacos metadata center imeplement.

[ralf0131]

@ralf0131 I'm fixing this. Also, I feel happy to help with nacos metadata center.

Cool, the estimated release date 2.7.2 is targeting 5.15, will you be able to finish it? If not I can help.
I am still testing the code with Nacos, I will update my result soon.

[Moriadry-zz]

Yes, I am going to finish this no later than the day after tomorrow :). Actually, if things not going well until tomorrow, I can contact with you.

[ralf0131]

In order to make dubbo-demo work, we also need to add the following code to dubbo-bom/pom.xml

+            <dependency>
+                <groupId>org.apache.dubbo</groupId>
+                <artifactId>dubbo-registry-nacos</artifactId>
+                <version>${project.version}</version>
+            </dependency>

[ralf0131]

Another issue I found is, when I create a service configurator via nacos-config, for example, dataId =dubbo-config-org.apache.dubbo.demo.DemoService.configurators and group=dubbo in the default namespace (known as public in Nacos), and run the dubbo-demo, the configuration cannot be retrieved.

The reason is, if the namespace is not specified, Dubbo use "dubbo" as a default namespace, while Nacos use empty string "" as default namespace. So what I did is to remove the url that contains parameter "namespace=dubbo". In that case, Nacos will use empty String instead.

Below is the change I made to NacosDynamicConfigurationFactory:

     protected DynamicConfiguration createDynamicConfiguration(URL url) {
-        return new NacosDynamicConfiguration(url);
+        URL nacosURL = url;
+        if (Constants.DUBBO.equals(url.getParameter(PropertyKeyConst.NAMESPACE))) {
+            // Nacos use empty string as default name space, replace default namespace "dubbo" to ""
+            nacosURL = url.removeParameter(PropertyKeyConst.NAMESPACE);
+        }
+        return new NacosDynamicConfiguration(nacosURL);
     }

[ralf0131]

Followed by the previous issue, if we configure dataId=dubbo-config-org.apache.dubbo.demo.DemoService.configurators and group=dubbo, Dubbo still can not retrieve the configuration value during start up, listeners to the update works well.

The reason is in org.apache.dubbo.registry.integration.AbstractConfiguratorListener#initWith, there is a call to get config directly:

String rawConfig = dynamicConfiguration.getConfig(key);

However, since no group is passed, the DEFAULT_GROUP will be used in Nacos, therefore the config value could not be retrieved.

Since in service governance configuration, Dubbo will always use "dubbo" as group. What I did is apply the following change to AbstractConfiguratorListener

-        String rawConfig = dynamicConfiguration.getConfig(key);
+        String rawConfig = dynamicConfiguration.getConfig(key, Constants.DUBBO);

then everything will work fine.

[Moriadry-zz]

Thank you Huxing! What's more, I want to add a util class in dubbo-common for nacos operation, to avoid some duplicated code. I can do this in nacos-metatada pull request. How do you think?

[ralf0131]

Thank you Huxing! Also, I want to add a util class in dubbo-common for nacos operation, to avoid some duplicated code.

Yeah, I notice some duplicated code in dubbo-registry-nacos and dubbo-configcenter-nacos. Do you mean that?

I can do this in nacos-metatada pull request. What do you think?

Look great!

[ralf0131]

Now the code looks good to me, thanks for the pull request!

[ralf0131]

Hi, I have already merged this pull request, looking forward to your metadata report implementation. :-)