浏览代码

Comparator constants in TransportGetSnapshotsAction (#106224)

Today we use a constant `Comparator<SnapshotInfo>` for ascending sorts,
but create a new instance for descending sorts each time. We should use
constants for both cases.
David Turner 1 年之前
父节点
当前提交
312c733e4f

+ 10 - 5
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/SnapshotSortKey.java

@@ -153,11 +153,13 @@ public enum SnapshotSortKey {
     };
 
     private final String name;
-    private final Comparator<SnapshotInfo> snapshotInfoComparator;
+    private final Comparator<SnapshotInfo> ascendingSnapshotInfoComparator;
+    private final Comparator<SnapshotInfo> descendingSnapshotInfoComparator;
 
     SnapshotSortKey(String name, Comparator<SnapshotInfo> snapshotInfoComparator) {
         this.name = name;
-        this.snapshotInfoComparator = snapshotInfoComparator.thenComparing(SnapshotInfo::snapshotId);
+        this.ascendingSnapshotInfoComparator = snapshotInfoComparator.thenComparing(SnapshotInfo::snapshotId);
+        this.descendingSnapshotInfoComparator = ascendingSnapshotInfoComparator.reversed();
     }
 
     @Override
@@ -166,10 +168,13 @@ public enum SnapshotSortKey {
     }
 
     /**
-     * @return a {@link Comparator} which can be used to sort {@link SnapshotInfo} items according to this sort key.
+     * @return a {@link Comparator} which sorts {@link SnapshotInfo} instances according to this sort key.
      */
-    public final Comparator<SnapshotInfo> getSnapshotInfoComparator() {
-        return snapshotInfoComparator;
+    public final Comparator<SnapshotInfo> getSnapshotInfoComparator(SortOrder sortOrder) {
+        return switch (sortOrder) {
+            case ASC -> ascendingSnapshotInfoComparator;
+            case DESC -> descendingSnapshotInfoComparator;
+        };
     }
 
     /**

+ 1 - 7
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java

@@ -54,7 +54,6 @@ import org.elasticsearch.transport.TransportService;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -529,7 +528,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
 
         private SnapshotsInRepo sortSnapshots(Stream<SnapshotInfo> snapshotInfoStream, int totalCount, int offset, int size) {
             final var resultsStream = snapshotInfoStream.filter(sortBy.getAfterPredicate(after, order))
-                .sorted(buildComparator())
+                .sorted(sortBy.getSnapshotInfoComparator(order))
                 .skip(offset);
             if (size == GetSnapshotsRequest.NO_LIMIT) {
                 return new SnapshotsInRepo(resultsStream.toList(), totalCount, 0);
@@ -548,11 +547,6 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                 return new SnapshotsInRepo(results, totalCount, remaining);
             }
         }
-
-        private Comparator<SnapshotInfo> buildComparator() {
-            final var comparator = sortBy.getSnapshotInfoComparator();
-            return order == SortOrder.DESC ? comparator.reversed() : comparator;
-        }
     }
 
     /**