Browse Source

Make sure there are comparators in InternalOrder.CompoundOrder (#63526)

If the constructor InternalOrder.CompoundOrder was called with an empty
compoundOrder, then comparator() returned a comparator that would crash
when used. This fails it early.
Radu Grigore 5 years ago
parent
commit
2e89e5dc67

+ 17 - 13
server/src/main/java/org/elasticsearch/search/aggregations/InternalOrder.java

@@ -34,7 +34,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
@@ -156,6 +155,9 @@ public abstract class InternalOrder extends BucketOrder {
                 // if all user provided comparators return 0.
                 this.orderElements.add(KEY_ASC);
             }
+            if (this.orderElements.isEmpty()) {
+                throw new IllegalArgumentException("empty compound order not supported");
+            }
         }
 
         @Override
@@ -185,12 +187,13 @@ public abstract class InternalOrder extends BucketOrder {
                     .map(oe -> oe.partiallyBuiltBucketComparator(ordinalReader, aggregator))
                     .collect(toList());
             return (lhs, rhs) -> {
-                Iterator<Comparator<T>> itr = comparators.iterator();
-                int result;
-                do {
-                    result = itr.next().compare(lhs, rhs);
-                } while (result == 0 && itr.hasNext());
-                return result;
+                for (Comparator<T> c : comparators) {
+                    int result = c.compare(lhs, rhs);
+                    if (result != 0) {
+                        return result;
+                    }
+                }
+                return 0;
             };
         }
 
@@ -198,12 +201,13 @@ public abstract class InternalOrder extends BucketOrder {
         public Comparator<Bucket> comparator() {
             List<Comparator<Bucket>> comparators = orderElements.stream().map(BucketOrder::comparator).collect(toList());
             return (lhs, rhs) -> {
-                Iterator<Comparator<Bucket>> itr = comparators.iterator();
-                int result;
-                do {
-                    result = itr.next().compare(lhs, rhs);
-                } while (result == 0 && itr.hasNext());
-                return result;
+                for (Comparator<Bucket> c : comparators) {
+                    int result = c.compare(lhs, rhs);
+                    if (result != 0) {
+                        return result;
+                    }
+                }
+                return 0;
             };
         }