fix #3069

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/RouterChain.java

@@ -18,6 +18,7 @@
 
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.common.extension.ExtensionLoader;
+import org.apache.dubbo.common.utils.CollectionUtils;
 import org.apache.dubbo.rpc.Invocation;
 import org.apache.dubbo.rpc.Invoker;
 
@@ -66,10 +67,6 @@ public void initWithRouters(List<Router> builtinRouters) {
         this.sort();
     }
 
-    public void addRouter(Router router) {
-        this.routers.add(router);
-    }
-
     /**
      * If we use route:// protocol in version before 2.7.0, each URL will generate a Router instance, so we should
      * keep the routers up to date, that is, each time router URLs changes, we should update the routers list, only
@@ -79,12 +76,11 @@ public void addRouter(Router router) {
      * @param routers routers from 'router://' rules in 2.6.x or before.
      */
     public void addRouters(List<Router> routers) {
-        // FIXME will sort cause concurrent problem? since it's kind of a write operation.
         List<Router> newRouters = new CopyOnWriteArrayList<>();
         newRouters.addAll(builtinRouters);
         newRouters.addAll(routers);
+        CollectionUtils.sort(routers);
         this.routers = newRouters;
-        this.sort();
     }
 
     private void sort() {
@@ -110,9 +106,7 @@ public List<Invoker<T>> route(URL url, Invocation invocation) {
      * Notify whenever addresses in registry change.
      */
     public void setInvokers(List<Invoker<T>> invokers) {
-        if (invokers != null) {
-            this.invokers = invokers;
-            routers.forEach(router -> router.notify(invokers));
-        }
+        this.invokers = (invokers == null ? Collections.emptyList() : invokers);
+        routers.forEach(router -> router.notify(this.invokers));
     }
 }

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/directory/AbstractDirectory.java

@@ -28,7 +28,7 @@
 import org.apache.dubbo.rpc.cluster.Router;
 import org.apache.dubbo.rpc.cluster.RouterChain;
 
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -96,8 +96,7 @@ public void setRouterChain(RouterChain<T> routerChain) {
     }
 
     protected void addRouters(List<Router> routers) {
-        // copy list
-        routers = routers == null ? new ArrayList<>() : new ArrayList<>(routers);
+        routers = routers == null ? Collections.emptyList() : routers;
         routerChain.addRouters(routers);
     }
 

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/condition/config/AppRouterFactory.java

@@ -29,7 +29,7 @@
 public class AppRouterFactory implements RouterFactory {
     public static final String NAME = "app";
 
-    private Router router;
+    private volatile Router router;
 
     @Override
     public Router getRouter(URL url) {

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/condition/config/ListenableRouter.java

@@ -20,8 +20,8 @@
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.common.logger.Logger;
 import org.apache.dubbo.common.logger.LoggerFactory;
-import org.apache.dubbo.common.utils.Assert;
 import org.apache.dubbo.common.utils.CollectionUtils;
+import org.apache.dubbo.common.utils.StringUtils;
 import org.apache.dubbo.configcenter.ConfigChangeEvent;
 import org.apache.dubbo.configcenter.ConfigChangeType;
 import org.apache.dubbo.configcenter.ConfigurationListener;
@@ -124,14 +124,15 @@ private void generateConditions(ConditionRouterRule rule, List<ConditionRouter>
         }
     }
 
-    private void init(String ruleKey) {
-        Assert.notEmptyString(ruleKey, "router rule's name cannot be null");
-        String router = ruleKey + Constants.ROUTERS_SUFFIX;
-        String rule = configuration.getConfig(router);
+    private synchronized void init(String ruleKey) {
+        if (StringUtils.isEmpty(ruleKey)) {
+            return;
+        }
+        String routerKey = ruleKey + Constants.ROUTERS_SUFFIX;
+        configuration.addListener(routerKey, this);
+        String rule = configuration.getConfig(routerKey);
         if (rule != null) {
-            this.process(new ConfigChangeEvent(router, rule));
+            this.process(new ConfigChangeEvent(routerKey, rule));
         }
-
-        configuration.addListener(router, this);
     }
 }

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/file/FileRouterFactory.java

@@ -53,8 +53,11 @@ public Router getRouter(URL url) {
             }
             String rule = IOUtils.read(new FileReader(new File(url.getAbsolutePath())));
 
+            // FIXME: this code looks useless
             boolean runtime = url.getParameter(Constants.RUNTIME_KEY, false);
-            URL script = url.setProtocol(protocol).addParameter(Constants.TYPE_KEY, type).addParameter(Constants.RUNTIME_KEY, runtime).addParameterAndEncoded(Constants.RULE_KEY, rule);
+            URL script = url.setProtocol(protocol).addParameter(Constants.TYPE_KEY, type)
+                    .addParameter(Constants.RUNTIME_KEY, runtime)
+                    .addParameterAndEncoded(Constants.RULE_KEY, rule);
 
             return routerFactory.getRouter(script);
         } catch (IOException e) {

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/script/ScriptRouter.java

@@ -45,7 +45,7 @@ public class ScriptRouter extends AbstractRouter {
     public static final String NAME = "SCRIPT_ROUTER";
     private static final Logger logger = LoggerFactory.getLogger(ScriptRouter.class);
 
-    private static final Map<String, ScriptEngine> engines = new ConcurrentHashMap<String, ScriptEngine>();
+    private static final Map<String, ScriptEngine> engines = new ConcurrentHashMap<>();
 
     private final ScriptEngine engine;
 
@@ -62,13 +62,13 @@ public ScriptRouter(URL url) {
             type = Constants.DEFAULT_SCRIPT_TYPE_KEY;
         }
         if (rule == null || rule.length() == 0) {
-            throw new IllegalStateException(new IllegalStateException("route rule can not be empty. rule:" + rule));
+            throw new IllegalStateException("route rule can not be empty. rule:" + rule);
         }
         ScriptEngine engine = engines.get(type);
         if (engine == null) {
             engine = new ScriptEngineManager().getEngineByName(type);
             if (engine == null) {
-                throw new IllegalStateException(new IllegalStateException("Unsupported route rule type: " + type + ", rule: " + rule));
+                throw new IllegalStateException("unsupported route rule type: " + type + ", rule: " + rule);
             }
             engines.put(type, engine);
         }
@@ -85,7 +85,7 @@ public URL getUrl() {
     @SuppressWarnings("unchecked")
     public <T> List<Invoker<T>> route(List<Invoker<T>> invokers, URL url, Invocation invocation) throws RpcException {
         try {
-            List<Invoker<T>> invokersCopy = new ArrayList<Invoker<T>>(invokers);
+            List<Invoker<T>> invokersCopy = new ArrayList<>(invokers);
             Compilable compilable = (Compilable) engine;
             Bindings bindings = engine.createBindings();
             bindings.put("invokers", invokersCopy);
@@ -105,8 +105,8 @@ public <T> List<Invoker<T>> route(List<Invoker<T>> invokers, URL url, Invocation
             }
             return invokersCopy;
         } catch (ScriptException e) {
-            //fail then ignore rule .invokers.
-            logger.error("route error , rule has been ignored. rule: " + rule + ", method:" + invocation.getMethodName() + ", url: " + RpcContext.getContext().getUrl(), e);
+            logger.error("route error, rule has been ignored. rule: " + rule + ", method:" +
+                    invocation.getMethodName() + ", url: " + RpcContext.getContext().getUrl(), e);
             return invokers;
         }
     }

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/TagRouter.java

@@ -42,7 +42,7 @@
 import static org.apache.dubbo.common.Constants.TAG_KEY;
 
 /**
- *
+ * TagRouter
  */
 public class TagRouter extends AbstractRouter implements Comparable<Router>, ConfigurationListener {
     public static final String NAME = "TAG_ROUTER";
@@ -117,10 +117,9 @@ public <T> List<Invoker<T>> route(List<Invoker<T>> invokers, URL url, Invocation
             }
             // FAILOVER: return all Providers without any tags.
             else {
-                List<Invoker<T>> tmp = filterInvoker(invokers, invoker -> addressNotMatches(invoker.getUrl(), tagRouterRule
-                        .getAddresses()));
-                return filterInvoker(tmp, invoker -> StringUtils.isEmpty(invoker.getUrl()
-                        .getParameter(TAG_KEY)));
+                List<Invoker<T>> tmp = filterInvoker(invokers, invoker -> addressNotMatches(invoker.getUrl(),
+                        tagRouterRule.getAddresses()));
+                return filterInvoker(tmp, invoker -> StringUtils.isEmpty(invoker.getUrl().getParameter(TAG_KEY)));
             }
         } else {
             // List<String> addresses = tagRouterRule.filter(providerApp);
@@ -137,10 +136,7 @@ public <T> List<Invoker<T>> route(List<Invoker<T>> invokers, URL url, Invocation
             }
             return filterInvoker(result, invoker -> {
                 String localTag = invoker.getUrl().getParameter(TAG_KEY);
-                if (StringUtils.isEmpty(localTag) || !tagRouterRule.getTagNames().contains(localTag)) {
-                    return true;
-                }
-                return false;
+                return StringUtils.isEmpty(localTag) || !tagRouterRule.getTagNames().contains(localTag);
             });
         }
     }

dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/router/file/FileRouterEngineTest.java

@@ -160,7 +160,8 @@ private void initInvokers(URL url, boolean invoker1Status, boolean invoker2Statu
 
     private void initDic(URL url) {
         // FIXME: this exposes the design flaw in RouterChain
-        dic = new StaticDirectory<>(url, invokers);
+        URL dicInitUrl = URL.valueOf("consumer://localhost:20880/org.apache.dubbo.rpc.cluster.router.file.FileRouterEngineTest?application=FileRouterEngineTest");
+        dic = new StaticDirectory<>(dicInitUrl, invokers);
         dic.buildRouterChain();
         dic.getRouterChain().initWithRouters(Arrays.asList(routerFactory.getRouter(url)));
     }

dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvokerTest.java

@@ -20,6 +20,7 @@
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.common.extension.ExtensionLoader;
 import org.apache.dubbo.common.utils.NetUtils;
+import org.apache.dubbo.common.utils.StringUtils;
 import org.apache.dubbo.rpc.Invocation;
 import org.apache.dubbo.rpc.Invoker;
 import org.apache.dubbo.rpc.Result;
@@ -61,7 +62,7 @@ public class AbstractClusterInvokerTest {
     AbstractClusterInvoker<IHelloService> cluster_nocheck;
     StaticDirectory<IHelloService> dic;
     RpcInvocation invocation = new RpcInvocation();
-    URL url = URL.valueOf("registry://localhost:9090/org.apache.dubbo.rpc.cluster.support.AbstractClusterInvokerTest.IHelloService");
+    URL url = URL.valueOf("registry://localhost:9090/org.apache.dubbo.rpc.cluster.support.AbstractClusterInvokerTest.IHelloService?refer=" + URL.encode("application=abstractClusterInvokerTest"));
 
     Invoker<IHelloService> invoker1;
     Invoker<IHelloService> invoker2;
@@ -221,8 +222,9 @@ public void testSelect_multiInvokers() throws Exception {
     @Test
     public void testCloseAvailablecheck() {
         LoadBalance lb = mock(LoadBalance.class);
-        given(lb.select(invokers, url, invocation)).willReturn(invoker1);
-
+        Map<String, String> queryMap = StringUtils.parseQueryString(url.getParameterAndDecoded(Constants.REFER_KEY));
+        URL tmpUrl = url.clearParameters().addParameters(queryMap).removeParameter(Constants.MONITOR_KEY);
+        given(lb.select(invokers, tmpUrl, invocation)).willReturn(invoker1);
         initlistsize5();
 
         Invoker sinvoker = cluster_nocheck.select(lb, invocation, invokers, selectedInvokers);

dubbo-common/src/main/java/org/apache/dubbo/common/async/support/AsyncFor.java

@@ -24,11 +24,11 @@
 
 /**
  * TODO This definition should better be placed in module 'dubbo-config-api', but only can be done when "rpc" dependencies are removed from "dubbo-config-api"
- * If an interface is annotated with AsyncFor, it will be treated as an async counterpart for the sync one.
+ * If a method is annotated with AsyncFor, it will be treated as an async counterpart for the sync specified in value.
  */
 @Documented
 @Retention(RetentionPolicy.RUNTIME)
-@Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE})
+@Target({ElementType.METHOD})
 public @interface AsyncFor {
 
     /**

dubbo-common/src/main/java/org/apache/dubbo/common/utils/ClassHelper.java

@@ -104,8 +104,7 @@ public static ClassLoader getClassLoader(Class<?> clazz) {
                 // getClassLoader() returning null indicates the bootstrap ClassLoader
                 try {
                     cl = ClassLoader.getSystemClassLoader();
-                }
-                catch (Throwable ex) {
+                } catch (Throwable ex) {
                     // Cannot access system ClassLoader - oh well, maybe the caller can live with null...
                 }
             }
@@ -265,10 +264,14 @@ public static boolean isPrimitive(Class<?> type) {
     }
 
     public static Object convertPrimitive(Class<?> type, String value) {
-        if (type == char.class || type == Character.class) {
+        if (value == null) {
+            return null;
+        } else if (type == char.class || type == Character.class) {
             return value.length() > 0 ? value.charAt(0) : '\0';
         } else if (type == boolean.class || type == Boolean.class) {
             return Boolean.valueOf(value);
+        } else if (!isNumber(value)) {
+            return null;
         } else if (type == byte.class || type == Byte.class) {
             return Byte.valueOf(value);
         } else if (type == short.class || type == Short.class) {
@@ -285,6 +288,11 @@ public static Object convertPrimitive(Class<?> type, String value) {
         return value;
     }
 
+    public static boolean isNumber(String str) {
+        String reg = "^[0-9]+(.[0-9]+)?$";
+        return str.matches(reg);
+    }
+
     /**
      * We only check boolean value at this moment.
      *

dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java

@@ -27,6 +27,14 @@
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
+import static org.apache.dubbo.common.Constants.CATEGORY_KEY;
+import static org.apache.dubbo.common.Constants.CONFIGURATORS_CATEGORY;
+import static org.apache.dubbo.common.Constants.DEFAULT_CATEGORY;
+import static org.apache.dubbo.common.Constants.OVERRIDE_PROTOCOL;
+import static org.apache.dubbo.common.Constants.PROVIDERS_CATEGORY;
+import static org.apache.dubbo.common.Constants.ROUTERS_CATEGORY;
+import static org.apache.dubbo.common.Constants.ROUTE_PROTOCOL;
+
 public class UrlUtils {
 
     /**
@@ -343,14 +351,14 @@ public static URL getEmptyUrl(String service, String category) {
             service = service.substring(0, i);
         }
         return URL.valueOf(Constants.EMPTY_PROTOCOL + "://0.0.0.0/" + service + URL_PARAM_STARTING_SYMBOL
-                + Constants.CATEGORY_KEY + "=" + category
+                + CATEGORY_KEY + "=" + category
                 + (group == null ? "" : "&" + Constants.GROUP_KEY + "=" + group)
                 + (version == null ? "" : "&" + Constants.VERSION_KEY + "=" + version));
     }
 
     public static boolean isMatchCategory(String category, String categories) {
         if (categories == null || categories.length() == 0) {
-            return Constants.DEFAULT_CATEGORY.equals(category);
+            return DEFAULT_CATEGORY.equals(category);
         } else if (categories.contains(Constants.ANY_VALUE)) {
             return true;
         } else if (categories.contains(Constants.REMOVE_VALUE_PREFIX)) {
@@ -370,8 +378,8 @@ public static boolean isMatch(URL consumerUrl, URL providerUrl) {
             return false;
         }
 
-        if (!isMatchCategory(providerUrl.getParameter(Constants.CATEGORY_KEY, Constants.DEFAULT_CATEGORY),
-                consumerUrl.getParameter(Constants.CATEGORY_KEY, Constants.DEFAULT_CATEGORY))) {
+        if (!isMatchCategory(providerUrl.getParameter(CATEGORY_KEY, DEFAULT_CATEGORY),
+                consumerUrl.getParameter(CATEGORY_KEY, DEFAULT_CATEGORY))) {
             return false;
         }
         if (!providerUrl.getParameter(Constants.ENABLED_KEY, true)
@@ -445,6 +453,22 @@ public static List<URL> classifyUrls(List<URL> urls, Predicate<URL> predicate) {
         return urls.stream().filter(predicate).collect(Collectors.toList());
     }
 
+    public static boolean isConfigurator(URL url) {
+        return OVERRIDE_PROTOCOL.equals(url.getProtocol()) ||
+                CONFIGURATORS_CATEGORY.equals(url.getParameter(CATEGORY_KEY, DEFAULT_CATEGORY));
+    }
+
+    public static boolean isRoute(URL url) {
+        return ROUTE_PROTOCOL.equals(url.getProtocol()) ||
+                ROUTERS_CATEGORY.equals(url.getParameter(CATEGORY_KEY, DEFAULT_CATEGORY));
+    }
+
+    public static boolean isProvider(URL url) {
+        return !OVERRIDE_PROTOCOL.equals(url.getProtocol()) &&
+                !ROUTE_PROTOCOL.equals(url.getProtocol()) &&
+                PROVIDERS_CATEGORY.equals(url.getParameter(CATEGORY_KEY, PROVIDERS_CATEGORY));
+    }
+
     /**
      * Check if the given value matches the given pattern. The pattern supports wildcard "*".
      *
@@ -459,4 +483,4 @@ static boolean isItemMatch(String pattern, String value) {
             return "*".equals(pattern) || pattern.equals(value);
         }
     }
-}
+}