fix order comparator overflow (#1150)

sofastack · Dec 22, 2021 · ebae921 · ebae921
1 parent 07ab43b
commit ebae921
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 15 deletions.
diff --git a/core/api/src/main/java/com/alipay/sofa/rpc/client/RouterChain.java b/core/api/src/main/java/com/alipay/sofa/rpc/client/RouterChain.java
@@ -17,7 +17,6 @@
 package com.alipay.sofa.rpc.client;
 
 import com.alipay.sofa.rpc.bootstrap.ConsumerBootstrap;
-import com.alipay.sofa.rpc.common.struct.OrderedComparator;
 import com.alipay.sofa.rpc.common.utils.CommonUtils;
 import com.alipay.sofa.rpc.common.utils.StringUtils;
 import com.alipay.sofa.rpc.config.ConsumerConfig;
@@ -31,6 +30,7 @@
 import com.alipay.sofa.rpc.log.LoggerFactory;
 
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.LinkedList;
 import java.util.Collections;
 import java.util.HashSet;
@@ -165,7 +165,7 @@ public static RouterChain buildConsumerChain(ConsumerBootstrap consumerBootstrap
         excludes = null; // 不需要了
         // 按order从小到大排序
         if (extensionRouters.size() > 1) {
-            Collections.sort(extensionRouters, new OrderedComparator<ExtensionClass>());
+            extensionRouters.sort(Comparator.comparingInt(ExtensionClass::getOrder));
         }
         List<Router> actualRouters = new ArrayList<Router>();
         for (ExtensionClass<Router> extensionRouter : extensionRouters) {

diff --git a/core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConfigs.java b/core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConfigs.java
@@ -19,7 +19,6 @@
 import com.alipay.sofa.rpc.base.Sortable;
 import com.alipay.sofa.rpc.common.annotation.JustForTest;
 import com.alipay.sofa.rpc.common.json.JSON;
-import com.alipay.sofa.rpc.common.struct.OrderedComparator;
 import com.alipay.sofa.rpc.common.utils.ClassLoaderUtils;
 import com.alipay.sofa.rpc.common.utils.CommonUtils;
 import com.alipay.sofa.rpc.common.utils.CompatibleTypeUtils;
@@ -32,7 +31,7 @@
 import java.io.InputStreamReader;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Comparator;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.List;
@@ -116,7 +115,7 @@ private static void loadCustom(String fileName) throws IOException {
                     }
                 }
             }
-            Collections.sort(allFile, new OrderedComparator<CfgFile>()); // 从小到大排下序
+            allFile.sort(Comparator.comparingInt(CfgFile::getOrder));
             for (CfgFile file : allFile) {
                 CFG.putAll(file.getMap()); // 顺序加载，越大越后加载
             }

diff --git a/core/api/src/main/java/com/alipay/sofa/rpc/filter/FilterChain.java b/core/api/src/main/java/com/alipay/sofa/rpc/filter/FilterChain.java
@@ -16,7 +16,6 @@
  */
 package com.alipay.sofa.rpc.filter;
 
-import com.alipay.sofa.rpc.common.struct.OrderedComparator;
 import com.alipay.sofa.rpc.common.utils.CommonUtils;
 import com.alipay.sofa.rpc.common.utils.StringUtils;
 import com.alipay.sofa.rpc.config.AbstractInterfaceConfig;
@@ -36,6 +35,7 @@
 import com.alipay.sofa.rpc.log.LoggerFactory;
 
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.LinkedList;
 import java.util.Collections;
 import java.util.HashSet;
@@ -214,7 +214,7 @@ private static List<Filter> selectActualFilters(AbstractInterfaceConfig config,
         }
         // 按order从小到大排序
         if (extensionFilters.size() > 1) {
-            Collections.sort(extensionFilters, new OrderedComparator<ExtensionClass<Filter>>());
+            extensionFilters.sort(Comparator.comparingInt(ExtensionClass::getOrder));
         }
         List<Filter> actualFilters = new ArrayList<Filter>();
         for (ExtensionClass<Filter> extensionFilter : extensionFilters) {

diff --git a/core/common/src/main/java/com/alipay/sofa/rpc/common/SofaConfigs.java b/core/common/src/main/java/com/alipay/sofa/rpc/common/SofaConfigs.java
@@ -17,13 +17,12 @@
 package com.alipay.sofa.rpc.common;
 
 import com.alipay.sofa.rpc.base.Sortable;
-import com.alipay.sofa.rpc.common.struct.OrderedComparator;
 import com.alipay.sofa.rpc.common.utils.CommonUtils;
 import com.alipay.sofa.rpc.common.utils.StringUtils;
 
 import java.io.InputStream;
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Properties;
 import java.util.concurrent.locks.Lock;
@@ -198,7 +197,7 @@ public static void registerExternalConfigLoader(ExternalConfigLoader configLoade
         wLock.lock();
         try {
             CONFIG_LOADERS.add(configLoader);
-            Collections.sort(CONFIG_LOADERS, new OrderedComparator<ExternalConfigLoader>());
+            CONFIG_LOADERS.sort(Comparator.comparingInt(ExternalConfigLoader::getOrder));
         } finally {
             wLock.unlock();
         }
@@ -213,7 +212,7 @@ public static void unRegisterExternalConfigLoader(ExternalConfigLoader configLoa
         wLock.lock();
         try {
             CONFIG_LOADERS.remove(configLoader);
-            Collections.sort(CONFIG_LOADERS, new OrderedComparator<ExternalConfigLoader>());
+            CONFIG_LOADERS.sort(Comparator.comparingInt(ExternalConfigLoader::getOrder));
         } finally {
             wLock.unlock();
         }

diff --git a/core/common/src/main/java/com/alipay/sofa/rpc/common/struct/OrderedComparator.java b/core/common/src/main/java/com/alipay/sofa/rpc/common/struct/OrderedComparator.java
@@ -27,6 +27,7 @@
  * @param <T> the type parameter
  * @author <a href="mailto:zhanggeng.zg@antfin.com">GengZhang</a>
  */
+@Deprecated
 public class OrderedComparator<T extends Sortable> implements Comparator<T>, Serializable {
 
     /**
@@ -53,7 +54,10 @@ public OrderedComparator(boolean smallToLarge) {
     @Override
     public int compare(T o1, T o2) {
         // order一样的情况下，顺序不变
-        return order ? o1.getOrder() - o2.getOrder() :
-            o2.getOrder() - o1.getOrder();
+        Comparator<Sortable> comparator = Comparator.comparingInt(Sortable::getOrder);
+        if (!order) {
+            comparator = comparator.reversed();
+        }
+        return comparator.compare(o1, o2);
     }
 }
diff --git a/core/common/src/test/java/com/alipay/sofa/rpc/common/struct/OrderedComparatorTest.java b/core/common/src/test/java/com/alipay/sofa/rpc/common/struct/OrderedComparatorTest.java
@@ -22,11 +22,10 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 
 /**
- *
- *
  * @author <a href="mailto:zhanggeng.zg@antfin.com">GengZhang</a>
  */
 public class OrderedComparatorTest {
@@ -57,6 +56,52 @@ public void testCompare() throws Exception {
         Assert.assertEquals(sb.toString(), "agcdbef");
     }
 
+    @Test
+    public void testComparatorInt(){
+        List<Obj> list = new ArrayList<Obj>();
+        list.add(new Obj("a", 10));
+        list.add(new Obj("b", 2));
+        list.add(new Obj("c", 6));
+        list.add(new Obj("d", 6));
+        list.add(new Obj("e", 0));
+        list.add(new Obj("f", -1));
+        list.add(new Obj("g", 10));
+
+        list.sort(Comparator.comparingInt(Obj::getOrder));
+        StringBuilder sb = new StringBuilder();
+        for (Obj test : list) {
+            sb.append(test.getName());
+        }
+        Assert.assertEquals(sb.toString(), "febcdag");
+
+
+        list.sort(Comparator.comparingInt(Obj::getOrder).reversed());
+        sb = new StringBuilder();
+        for (Obj test : list) {
+            sb.append(test.getName());
+        }
+        Assert.assertEquals(sb.toString(), "agcdbef");
+    }
+
+    @Test
+    public void testOverflow() {
+        List<Obj> list = new ArrayList<Obj>();
+        list.add(new Obj("a", Integer.MAX_VALUE));
+        list.add(new Obj("b", -100));
+        list.add(new Obj("c", 6));
+        list.add(new Obj("d", 6));
+        list.add(new Obj("e", 0));
+        list.add(new Obj("f", Integer.MAX_VALUE));
+        list.add(new Obj("g", -10));
+
+        Collections.sort(list, new OrderedComparator<Obj>());
+        StringBuilder sb = new StringBuilder();
+        for (Obj test : list) {
+            sb.append(test.getName());
+        }
+        Assert.assertEquals(sb.toString(), "bgecdaf");
+    }
+
     private static class Obj implements Sortable {
         private final String name;
         private final int    order;