Optimize compareTo in Router to guarantee consistent behaviour.

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

@@ -32,6 +32,9 @@
  * @see org.apache.dubbo.rpc.cluster.Directory#list(Invocation)
  */
 public interface Router extends Comparable<Router> {
+
+    int DEFAULT_PRIORITY = Integer.MAX_VALUE;
+
     /**
      * Get the router url.
      *
@@ -91,16 +94,6 @@ default int compareTo(Router o) {
         if (o == null) {
             throw new IllegalArgumentException();
         }
-        if (this.getPriority() == o.getPriority()) {
-            if (o.getUrl() == null) {
-                return 1;
-            }
-            if (getUrl() == null) {
-                return -1;
-            }
-            return getUrl().toFullString().compareTo(o.getUrl().toFullString());
-        } else {
-            return getPriority() > o.getPriority() ? 1 : -1;
-        }
+        return Integer.compare(this.getPriority(), o.getPriority());
     }
 }

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

@@ -21,7 +21,7 @@
 import org.apache.dubbo.rpc.cluster.Router;
 
 public abstract class AbstractRouter implements Router {
-    protected int priority;
+    protected int priority = DEFAULT_PRIORITY;
     protected boolean force = false;
     protected URL url;
 

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

@@ -27,7 +27,6 @@
 import org.apache.dubbo.rpc.Invocation;
 import org.apache.dubbo.rpc.Invoker;
 import org.apache.dubbo.rpc.RpcException;
-import org.apache.dubbo.rpc.cluster.Router;
 import org.apache.dubbo.rpc.cluster.router.AbstractRouter;
 
 import java.text.ParseException;
@@ -44,7 +43,7 @@
  * ConditionRouter
  *
  */
-public class ConditionRouter extends AbstractRouter implements Comparable<Router> {
+public class ConditionRouter extends AbstractRouter {
     public static final String NAME = "condition";
 
     private static final Logger logger = LoggerFactory.getLogger(ConditionRouter.class);

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

@@ -25,8 +25,13 @@
  */
 public class AppRouter extends ListenableRouter {
     public static final String NAME = "APP_ROUTER";
+    /**
+     * AppRouter should after ServiceRouter
+     */
+    private static final int APP_ROUTER_DEFAULT_PRIORITY = 150;
 
     public AppRouter(DynamicConfiguration configuration, URL url) {
         super(configuration, url, url.getParameter(Constants.APPLICATION_KEY));
+        this.priority = APP_ROUTER_DEFAULT_PRIORITY;
     }
 }

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

@@ -44,7 +44,7 @@
 public abstract class ListenableRouter extends AbstractRouter implements ConfigurationListener {
     public static final String NAME = "LISTENABLE_ROUTER";
     private static final String RULE_SUFFIX = ".condition-router";
-    public static final int DEFAULT_PRIORITY = 200;
+
     private static final Logger logger = LoggerFactory.getLogger(ListenableRouter.class);
     private ConditionRouterRule routerRule;
     private List<ConditionRouter> conditionRouters = Collections.emptyList();

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

@@ -24,8 +24,13 @@
  */
 public class ServiceRouter extends ListenableRouter {
     public static final String NAME = "SERVICE_ROUTER";
+    /**
+     * ServiceRouter should before AppRouter
+     */
+    private static final int SERVICE_ROUTER_DEFAULT_PRIORITY = 140;
 
     public ServiceRouter(DynamicConfiguration configuration, URL url) {
         super(configuration, url, url.getEncodedServiceKey());
+        this.priority = SERVICE_ROUTER_DEFAULT_PRIORITY;
     }
 }

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/mock/MockInvokersSelector.java

@@ -30,11 +30,15 @@
 /**
  * A specific Router designed to realize mock feature.
  * If a request is configured to use mock, then this router guarantees that only the invokers with protocol MOCK appear in final the invoker list, all other invokers will be excluded.
- *
  */
 public class MockInvokersSelector extends AbstractRouter {
 
     public static final String NAME = "MOCK_ROUTER";
+    private static final int MOCK_INVOKERS_DEFAULT_PRIORITY = Integer.MIN_VALUE;
+
+    public MockInvokersSelector() {
+        this.priority = MOCK_INVOKERS_DEFAULT_PRIORITY;
+    }
 
     @Override
     public <T> List<Invoker<T>> route(final List<Invoker<T>> invokers,
@@ -94,9 +98,4 @@ private <T> boolean hasMockProviders(final List<Invoker<T>> invokers) {
         return hasMockProvider;
     }
 
-    @Override
-    public int getPriority() {
-        return Integer.MAX_VALUE;
-    }
-
 }

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

@@ -25,7 +25,6 @@
 import org.apache.dubbo.rpc.Invoker;
 import org.apache.dubbo.rpc.RpcContext;
 import org.apache.dubbo.rpc.RpcException;
-import org.apache.dubbo.rpc.cluster.Router;
 import org.apache.dubbo.rpc.cluster.router.AbstractRouter;
 
 import javax.script.Bindings;
@@ -46,6 +45,7 @@
  */
 public class ScriptRouter extends AbstractRouter {
     public static final String NAME = "SCRIPT_ROUTER";
+    private static final int SCRIPT_ROUTER_DEFAULT_PRIORITY = 0;
     private static final Logger logger = LoggerFactory.getLogger(ScriptRouter.class);
 
     private static final Map<String, ScriptEngine> engines = new ConcurrentHashMap<>();
@@ -58,7 +58,7 @@ public class ScriptRouter extends AbstractRouter {
 
     public ScriptRouter(URL url) {
         this.url = url;
-        this.priority = url.getParameter(Constants.PRIORITY_KEY, 0);
+        this.priority = url.getParameter(Constants.PRIORITY_KEY, SCRIPT_ROUTER_DEFAULT_PRIORITY);
 
         engine = getEngine(url);
         rule = getRule(url);
@@ -150,12 +150,4 @@ public boolean isForce() {
         return url.getParameter(Constants.FORCE_KEY, false);
     }
 
-    @Override
-    public int compareTo(Router o) {
-        if (o == null || o.getClass() != ScriptRouter.class) {
-            return 1;
-        }
-        ScriptRouter c = (ScriptRouter) o;
-        return this.priority == c.priority ? rule.compareTo(c.rule) : (this.priority > c.priority ? 1 : -1);
-    }
 }

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

@@ -29,7 +29,6 @@
 import org.apache.dubbo.rpc.Invocation;
 import org.apache.dubbo.rpc.Invoker;
 import org.apache.dubbo.rpc.RpcException;
-import org.apache.dubbo.rpc.cluster.Router;
 import org.apache.dubbo.rpc.cluster.router.AbstractRouter;
 import org.apache.dubbo.rpc.cluster.router.tag.model.TagRouterRule;
 import org.apache.dubbo.rpc.cluster.router.tag.model.TagRuleParser;
@@ -44,9 +43,9 @@
 /**
  * TagRouter, "application.tag-router"
  */
-public class TagRouter extends AbstractRouter implements Comparable<Router>, ConfigurationListener {
+public class TagRouter extends AbstractRouter implements ConfigurationListener {
     public static final String NAME = "TAG_ROUTER";
-    private static final int DEFAULT_PRIORITY = 100;
+    private static final int TAG_ROUTER_DEFAULT_PRIORITY = 100;
     private static final Logger logger = LoggerFactory.getLogger(TagRouter.class);
     private static final String RULE_SUFFIX = ".tag-router";
 
@@ -55,6 +54,7 @@ public class TagRouter extends AbstractRouter implements Comparable<Router>, Con
 
     public TagRouter(DynamicConfiguration configuration, URL url) {
         super(configuration, url);
+        this.priority = TAG_ROUTER_DEFAULT_PRIORITY;
     }
 
     @Override
@@ -172,11 +172,6 @@ private <T> List<Invoker<T>> filterUsingStaticTag(List<Invoker<T>> invokers, URL
         return result;
     }
 
-    @Override
-    public int getPriority() {
-        return DEFAULT_PRIORITY;
-    }
-
     @Override
     public boolean isRuntime() {
         return tagRouterRule != null && tagRouterRule.isRuntime();