Simplify the code logic of the method AbstractClusterInvoker#reselect. (#2826)

* Simplify the code logic of the method AbstractClusterInvoker#reselect.

* Minor modification for code style.

Author: code4wt

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java

@@ -24,6 +24,7 @@
 import org.apache.dubbo.common.logger.LoggerFactory;
 import org.apache.dubbo.common.utils.CollectionUtils;
 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;
@@ -44,8 +45,8 @@
  */
 public abstract class AbstractClusterInvoker<T> implements Invoker<T> {
 
-    private static final Logger logger = LoggerFactory
-            .getLogger(AbstractClusterInvoker.class);
+    private static final Logger logger = LoggerFactory.getLogger(AbstractClusterInvoker.class);
+
     protected final Directory<T> directory;
 
     protected final boolean availablecheck;
@@ -110,13 +111,16 @@ public void destroy() {
      * @return the invoker which will final to do invoke.
      * @throws RpcException
      */
-    protected Invoker<T> select(LoadBalance loadbalance, Invocation invocation, List<Invoker<T>> invokers, List<Invoker<T>> selected) throws RpcException {
-        if (invokers == null || invokers.isEmpty()) {
+    protected Invoker<T> select(LoadBalance loadbalance, Invocation invocation,
+        List<Invoker<T>> invokers, List<Invoker<T>> selected) throws RpcException {
+
+        if (CollectionUtils.isEmpty(invokers)) {
             return null;
         }
-        String methodName = invocation == null ? "" : invocation.getMethodName();
+        String methodName = invocation == null ? StringUtils.EMPTY : invocation.getMethodName();
 
-        boolean sticky = invokers.get(0).getUrl().getMethodParameter(methodName, Constants.CLUSTER_STICKY_KEY, Constants.DEFAULT_CLUSTER_STICKY);
+        boolean sticky = invokers.get(0).getUrl()
+            .getMethodParameter(methodName, Constants.CLUSTER_STICKY_KEY, Constants.DEFAULT_CLUSTER_STICKY);
         {
             //ignore overloaded method
             if (stickyInvoker != null && !invokers.contains(stickyInvoker)) {
@@ -137,8 +141,10 @@ protected Invoker<T> select(LoadBalance loadbalance, Invocation invocation, List
         return invoker;
     }
 
-    private Invoker<T> doSelect(LoadBalance loadbalance, Invocation invocation, List<Invoker<T>> invokers, List<Invoker<T>> selected) throws RpcException {
-        if (invokers == null || invokers.isEmpty()) {
+    private Invoker<T> doSelect(LoadBalance loadbalance, Invocation invocation,
+        List<Invoker<T>> invokers, List<Invoker<T>> selected) throws RpcException {
+
+        if (CollectionUtils.isEmpty(invokers)) {
             return null;
         }
         if (invokers.size() == 1) {
@@ -158,7 +164,7 @@ private Invoker<T> doSelect(LoadBalance loadbalance, Invocation invocation, List
                     int index = invokers.indexOf(invoker);
                     try {
                         //Avoid collision
-                        invoker = index < invokers.size() - 1 ? invokers.get(index + 1) : invokers.get(0);
+                        invoker = invokers.get((index + 1) % invokers.size());
                     } catch (Exception e) {
                         logger.warn(e.getMessage() + " may because invokers list dynamic change, ignore.", e);
                     }
@@ -171,7 +177,8 @@ private Invoker<T> doSelect(LoadBalance loadbalance, Invocation invocation, List
     }
 
     /**
-     * Reselect, use invokers not in `selected` first, if all invokers are in `selected`, just pick an available one using loadbalance policy.
+     * Reselect, use invokers not in `selected` first, if all invokers are in `selected`,
+     * just pick an available one using loadbalance policy.
      *
      * @param loadbalance
      * @param invocation
@@ -181,34 +188,27 @@ private Invoker<T> doSelect(LoadBalance loadbalance, Invocation invocation, List
      * @throws RpcException
      */
     private Invoker<T> reselect(LoadBalance loadbalance, Invocation invocation,
-                                List<Invoker<T>> invokers, List<Invoker<T>> selected, boolean availablecheck)
-            throws RpcException {
+        List<Invoker<T>> invokers, List<Invoker<T>> selected, boolean availablecheck) throws RpcException {
 
         //Allocating one in advance, this list is certain to be used.
-        List<Invoker<T>> reselectInvokers = new ArrayList<Invoker<T>>(invokers.size() > 1 ? (invokers.size() - 1) : invokers.size());
+        List<Invoker<T>> reselectInvokers = new ArrayList<>(
+            invokers.size() > 1 ? (invokers.size() - 1) : invokers.size());
 
-        //First, try picking a invoker not in `selected`.
-        if (availablecheck) { // invoker.isAvailable() should be checked
-            for (Invoker<T> invoker : invokers) {
-                if (invoker.isAvailable()) {
-                    if (selected == null || !selected.contains(invoker)) {
-                        reselectInvokers.add(invoker);
-                    }
-                }
-            }
-            if (!reselectInvokers.isEmpty()) {
-                return loadbalance.select(reselectInvokers, getUrl(), invocation);
+        // First, try picking a invoker not in `selected`.
+        for (Invoker<T> invoker : invokers) {
+            if (availablecheck && !invoker.isAvailable()) {
+                continue;
             }
-        } else { // do not check invoker.isAvailable()
-            for (Invoker<T> invoker : invokers) {
-                if (selected == null || !selected.contains(invoker)) {
-                    reselectInvokers.add(invoker);
-                }
-            }
-            if (!reselectInvokers.isEmpty()) {
-                return loadbalance.select(reselectInvokers, getUrl(), invocation);
+
+            if (selected == null || !selected.contains(invoker)) {
+                reselectInvokers.add(invoker);
             }
         }
+
+        if (!reselectInvokers.isEmpty()) {
+            return loadbalance.select(reselectInvokers, getUrl(), invocation);
+        }
+
         // Just pick an available invoker using loadbalance policy
         {
             if (selected != null) {
@@ -257,7 +257,7 @@ public String toString() {
     }
 
     protected void checkInvokers(List<Invoker<T>> invokers, Invocation invocation) {
-        if (invokers == null || invokers.isEmpty()) {
+        if (CollectionUtils.isEmpty(invokers)) {
             throw new RpcException("Failed to invoke the method "
                     + invocation.getMethodName() + " in the service " + getInterface().getName()
                     + ". No provider available for the service " + directory.getUrl().getServiceKey()

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java

@@ -65,7 +65,7 @@ public FailbackClusterInvoker(Directory<T> directory) {
         super(directory);
     }
 
-    private void addFailed(Invocation invocation, AbstractClusterInvoker<?> router) {
+    private void addFailed(Invocation invocation, AbstractClusterInvoker<?> invoker) {
         if (retryFuture == null) {
             synchronized (this) {
                 if (retryFuture == null) {
@@ -84,11 +84,11 @@ public void run() {
                 }
             }
         }
-        failed.put(invocation, router);
+        failed.put(invocation, invoker);
     }
 
     void retryFailed() {
-        if (failed.size() == 0) {
+        if (failed.isEmpty()) {
             return;
         }
         for (Map.Entry<Invocation, AbstractClusterInvoker<?>> entry : new HashMap<>(failed).entrySet()) {