[Enhancement] Improve code coverage and fix some code (#6197)

dubbo-common/src/main/java/org/apache/dubbo/common/BaseServiceMetadata.java

@@ -22,7 +22,7 @@
  * 2019-10-10
  */
 public class BaseServiceMetadata {
-    public static final char COLON_SEPERATOR = ':';
+    public static final char COLON_SEPARATOR = ':';
 
     protected String serviceKey;
     protected String serviceInterfaceName;
@@ -73,7 +73,7 @@ public static String interfaceFromServiceKey(String serviceKey) {
     public String getDisplayServiceKey() {
         StringBuilder serviceNameBuilder = new StringBuilder();
         serviceNameBuilder.append(serviceInterfaceName);
-        serviceNameBuilder.append(COLON_SEPERATOR).append(version);
+        serviceNameBuilder.append(COLON_SEPARATOR).append(version);
         return serviceNameBuilder.toString();
     }
 
@@ -84,7 +84,7 @@ public String getDisplayServiceKey() {
      * @return
      */
     public static BaseServiceMetadata revertDisplayServiceKey(String displayKey) {
-        String[] eles = StringUtils.split(displayKey, COLON_SEPERATOR);
+        String[] eles = StringUtils.split(displayKey, COLON_SEPARATOR);
         if (eles == null || eles.length < 1 || eles.length > 2) {
             return new BaseServiceMetadata();
         }

dubbo-common/src/main/java/org/apache/dubbo/common/URLStrParser.java

@@ -159,7 +159,7 @@ private static URL parseURLBody(String fullURLStr, String decodedBody, Map<Strin
      */
     public static URL parseEncodedStr(String encodedURLStr) {
         Map<String, String> parameters = null;
-        int pathEndIdx = encodedURLStr.indexOf("%3F");// '?'
+        int pathEndIdx = encodedURLStr.toUpperCase().indexOf("%3F");// '?'
         if (pathEndIdx >= 0) {
             parameters = parseEncodedParams(encodedURLStr, pathEndIdx + 3);
         } else {

dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/ThreadlessExecutor.java

@@ -32,7 +32,7 @@
 /**
  * The most important difference between this Executor and other normal Executor is that this one doesn't manage
  * any thread.
- *
+ * <p>
  * Tasks submitted to this executor through {@link #execute(Runnable)} will not get scheduled to a specific thread, though normal executors always do the schedule.
  * Those tasks are stored in a blocking queue and will only be executed when a thread calls {@link #waitAndDrain()}, the thread executing the task
  * is exactly the same as the one calling waitAndDrain.
@@ -95,12 +95,7 @@ public void waitAndDrain() throws InterruptedException {
 
         runnable = queue.poll();
         while (runnable != null) {
-            try {
-                runnable.run();
-            } catch (Throwable t) {
-                logger.info(t);
-
-            }
+            runnable.run();
             runnable = queue.poll();
         }
         // mark the status of ThreadlessExecutor as finished.
@@ -131,6 +126,7 @@ public long waitAndDrain(long timeout, TimeUnit unit) throws InterruptedExceptio
      */
     @Override
     public void execute(Runnable runnable) {
+        runnable = new RunnableWrapper(runnable);
         synchronized (lock) {
             if (!waiting) {
                 sharedExecutor.execute(runnable);
@@ -180,4 +176,21 @@ public boolean isTerminated() {
     public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException {
         return false;
     }
+
+    private static class RunnableWrapper implements Runnable {
+        private Runnable runnable;
+
+        public RunnableWrapper(Runnable runnable) {
+            this.runnable = runnable;
+        }
+
+        @Override
+        public void run() {
+            try {
+                runnable.run();
+            } catch (Throwable t) {
+                logger.info(t);
+            }
+        }
+    }
 }

dubbo-common/src/test/java/org/apache/dubbo/common/BaseServiceMetadataTest.java

@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.common;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+public class BaseServiceMetadataTest {
+
+    @Test
+    public void test() {
+        BaseServiceMetadata baseServiceMetadata = new BaseServiceMetadata();
+        baseServiceMetadata.setGroup("group1");
+        baseServiceMetadata.setServiceInterfaceName("org.apache.dubbo.common.TestInterface");
+        baseServiceMetadata.setVersion("1.0.0");
+        baseServiceMetadata.setServiceKey(BaseServiceMetadata.buildServiceKey("org.apache.dubbo.common.TestInterface", "group1", "1.0.0"));
+
+        assertEquals(baseServiceMetadata.getGroup(), "group1");
+        assertEquals(baseServiceMetadata.getServiceInterfaceName(), "org.apache.dubbo.common.TestInterface");
+        assertEquals(baseServiceMetadata.getVersion(), "1.0.0");
+        assertEquals(baseServiceMetadata.getServiceKey(), "group1/org.apache.dubbo.common.TestInterface:1.0.0");
+        assertEquals(baseServiceMetadata.getDisplayServiceKey(), "org.apache.dubbo.common.TestInterface:1.0.0");
+
+        baseServiceMetadata.setServiceKey(BaseServiceMetadata.buildServiceKey("org.apache.dubbo.common.TestInterface", null, null));
+        assertEquals(baseServiceMetadata.getServiceKey(), "org.apache.dubbo.common.TestInterface");
+        baseServiceMetadata.setServiceKey(BaseServiceMetadata.buildServiceKey("org.apache.dubbo.common.TestInterface", "", ""));
+        assertEquals(baseServiceMetadata.getServiceKey(), "org.apache.dubbo.common.TestInterface");
+
+
+        baseServiceMetadata.setVersion("2.0.0");
+        baseServiceMetadata.generateServiceKey();
+        assertEquals(baseServiceMetadata.getServiceKey(), "group1/org.apache.dubbo.common.TestInterface:2.0.0");
+
+        assertEquals(BaseServiceMetadata.versionFromServiceKey("group1/org.apache.dubbo.common.TestInterface:1.0.0"), "1.0.0");
+        assertEquals(BaseServiceMetadata.groupFromServiceKey("group1/org.apache.dubbo.common.TestInterface:1.0.0"), "group1");
+        assertEquals(BaseServiceMetadata.interfaceFromServiceKey("group1/org.apache.dubbo.common.TestInterface:1.0.0"), "org.apache.dubbo.common.TestInterface");
+
+        assertNull(BaseServiceMetadata.versionFromServiceKey(""));
+        assertNull(BaseServiceMetadata.groupFromServiceKey(""));
+        assertEquals(BaseServiceMetadata.interfaceFromServiceKey(""), "");
+
+        assertEquals(BaseServiceMetadata.revertDisplayServiceKey("org.apache.dubbo.common.TestInterface:1.0.0").getDisplayServiceKey(),
+                "org.apache.dubbo.common.TestInterface:1.0.0");
+        assertEquals(BaseServiceMetadata.revertDisplayServiceKey("org.apache.dubbo.common.TestInterface").getDisplayServiceKey(),
+                "org.apache.dubbo.common.TestInterface:null");
+        assertEquals(BaseServiceMetadata.revertDisplayServiceKey(null).getDisplayServiceKey(),"null:null");
+        assertEquals(BaseServiceMetadata.revertDisplayServiceKey("org.apache.dubbo.common.TestInterface:1.0.0:1").getDisplayServiceKey(),"null:null");
+    }
+}

dubbo-common/src/test/java/org/apache/dubbo/common/URLStrParserTest.java

@@ -16,25 +16,67 @@
  */
 package org.apache.dubbo.common;
 
+import net.bytebuddy.utility.RandomString;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
 
 /**
  * Created by LinShunkang on 2020/03/12
  */
 public class URLStrParserTest {
+    private static Set<String> testCases = new HashSet<>(16);
+    private static Set<String> errorDecodedCases = new HashSet<>(8);
+    private static Set<String> errorEncodedCases = new HashSet<>(8);
+
+    static {
+        testCases.add("dubbo://192.168.1.1");
+        testCases.add("dubbo://192.168.1.1?");
+        testCases.add("dubbo://127.0.0.1?test=中文测试");
+        testCases.add("dubbo://admin:admin123@192.168.1.41:28113/org.test.api.DemoService$Iface?anyhost=true&application=demo-service&dubbo=2.6.1&generic=false&interface=org.test.api.DemoService$Iface&methods=orbCompare,checkText,checkPicture&pid=65557&revision=1.4.17&service.filter=bootMetrics&side=provider&status=server&threads=200&timestamp=1583136298859&version=1.0.0");
+        // super long text test
+        testCases.add("dubbo://192.168.1.1/" + RandomString.make(10240));
+        testCases.add("file:/path/to/file.txt");
+        testCases.add("dubbo://fe80:0:0:0:894:aeec:f37d:23e1%en0/path?abc=abc");
+
+        errorDecodedCases.add("dubbo:192.168.1.1");
+        errorDecodedCases.add("://192.168.1.1");
+        errorDecodedCases.add(":/192.168.1.1");
+
+        errorEncodedCases.add("dubbo%3a%2f%2f192.168.1.41%3fabc%3");
+        errorEncodedCases.add("dubbo%3a192.168.1.1%3fabc%3dabc");
+        errorEncodedCases.add("%3a%2f%2f192.168.1.1%3fabc%3dabc");
+        errorEncodedCases.add("%3a%2f192.168.1.1%3fabc%3dabc");
+        errorEncodedCases.add("dubbo%3a%2f%2f127.0.0.1%3ftest%3d%e2%96%b2%e2%96%bc%e2%97%80%e2%96%b6%e2%86%90%e2%86%91%e2%86%92%e2%86%93%e2%86%94%e2%86%95%e2%88%9e%c2%b1%e9%be%98%e9%9d%90%e9%bd%89%9%d%b");
+    }
 
     @Test
-    public void test() {
-        String str = "dubbo%3A%2F%2Fadmin%3Aadmin123%40192.168.1.41%3A28113%2Forg.test.api.DemoService%24Iface%3Fanyhost%3Dtrue%26application%3Ddemo-service%26dubbo%3D2.6.1%26generic%3Dfalse%26interface%3Dorg.test.api.DemoService%24Iface%26methods%3DorbCompare%2CcheckText%2CcheckPicture%26pid%3D65557%26revision%3D1.4.17%26service.filter%3DbootMetrics%26side%3Dprovider%26status%3Dserver%26threads%3D200%26timestamp%3D1583136298859%26version%3D1.0.0";
-        System.out.println(URLStrParser.parseEncodedStr(str));
-
-        String decodeStr = URL.decode(str);
-        URL originalUrl = URL.valueOf(decodeStr);
-        assertThat(URLStrParser.parseEncodedStr(str), equalTo(originalUrl));
-        assertThat(URLStrParser.parseDecodedStr(decodeStr), equalTo(originalUrl));
+    public void testEncoded() {
+        testCases.forEach(testCase -> {
+            assertThat(URLStrParser.parseEncodedStr(URL.encode(testCase)), equalTo(URL.valueOf(testCase)));
+        });
+
+        errorEncodedCases.forEach(errorCase -> {
+            Assertions.assertThrows(RuntimeException.class,
+                    () -> URLStrParser.parseEncodedStr(errorCase));
+        });
+    }
+
+    @Test
+    public void testDecoded() {
+        testCases.forEach(testCase -> {
+            assertThat(URLStrParser.parseDecodedStr(testCase), equalTo(URL.valueOf(testCase)));
+        });
+
+        errorDecodedCases.forEach(errorCase -> {
+            Assertions.assertThrows(RuntimeException.class,
+                    () -> URLStrParser.parseDecodedStr(errorCase));
+        });
     }
 
 }

dubbo-common/src/test/java/org/apache/dubbo/common/threadpool/ThreadlessExecutorTest.java

@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.common.threadpool;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.extension.ExtensionLoader;
+import org.apache.dubbo.common.threadpool.manager.ExecutorRepository;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutorService;
+
+public class ThreadlessExecutorTest {
+    private static ThreadlessExecutor executor;
+
+    static {
+        URL url = URL.valueOf("dubbo://127.0.0.1:12345");
+        ExecutorService sharedExecutor =
+                ExtensionLoader.getExtensionLoader(ExecutorRepository.class)
+                        .getDefaultExtension().createExecutorIfAbsent(url);
+        executor = new ThreadlessExecutor(sharedExecutor);
+    }
+
+    @Test
+    public void test() throws InterruptedException {
+        for (int i = 0; i < 10; i++) {
+            executor.execute(()->{throw new RuntimeException("test");});
+        }
+
+        CompletableFuture<Object> stubFuture = new CompletableFuture<>();
+        executor.setWaitingFuture(stubFuture);
+        Assertions.assertEquals(executor.getWaitingFuture(),stubFuture);
+
+        executor.waitAndDrain();
+
+        executor.execute(()->{});
+
+        executor.waitAndDrain();
+
+        executor.shutdown();
+    }
+}

dubbo-common/src/test/java/org/apache/dubbo/common/threadpool/manager/ExecutorRepositoryTest.java

@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.common.threadpool.manager;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.extension.ExtensionLoader;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadPoolExecutor;
+
+public class ExecutorRepositoryTest {
+    private ExecutorRepository executorRepository = ExtensionLoader.getExtensionLoader(ExecutorRepository.class).getDefaultExtension();
+
+    @Test
+    public void testGetExecutor() {
+        testGet(URL.valueOf("dubbo://127.0.0.1:23456"));
+        testGet(URL.valueOf("dubbo://127.0.0.1:23456?side=consumer"));
+
+        Assertions.assertNotNull(executorRepository.getSharedExecutor());
+        Assertions.assertNotNull(executorRepository.getServiceExporterExecutor());
+        executorRepository.nextScheduledExecutor();
+    }
+
+    private void testGet(URL url) {
+        Assertions.assertNull(executorRepository.getExecutor(url));
+
+        ExecutorService executorService = executorRepository.createExecutorIfAbsent(url);
+        executorService.shutdown();
+        executorService = executorRepository.createExecutorIfAbsent(url);
+        Assertions.assertFalse(executorService.isShutdown());
+
+        Assertions.assertEquals(executorService, executorRepository.getExecutor(url));
+        executorService.shutdown();
+        Assertions.assertNotEquals(executorService, executorRepository.getExecutor(url));
+    }
+
+    @Test
+    public void testUpdateExecutor() {
+        URL url = URL.valueOf("dubbo://127.0.0.1:23456?threads=5");
+        ThreadPoolExecutor executorService = (ThreadPoolExecutor) executorRepository.createExecutorIfAbsent(url);
+
+        executorService.setCorePoolSize(3);
+        executorRepository.updateThreadpool(url, executorService);
+
+        executorService.setCorePoolSize(3);
+        executorService.setMaximumPoolSize(3);
+        executorRepository.updateThreadpool(url, executorService);
+
+        executorService.setMaximumPoolSize(20);
+        executorService.setCorePoolSize(10);
+        executorRepository.updateThreadpool(url, executorService);
+
+        executorService.setCorePoolSize(10);
+        executorService.setMaximumPoolSize(10);
+        executorRepository.updateThreadpool(url, executorService);
+
+        executorService.setCorePoolSize(5);
+        executorRepository.updateThreadpool(url, executorService);
+
+
+    }
+}