Browse Source

Priority values should be unmodifiable

In Priority there is a field named values that represents an ordered, by
priority, list of all priorities. Yet, this collection is modifiable and
this collection is exposed via the public API. This means that consumers
can modify this list potentially leading to complete chaos. This commit
modifies this field so that it is unmodifiable, documents that the
returned collection is unmodifiable, and returns total order to the
world. We also punish the bad consumer here by making them make a copy
of the returned collection with which they can do as they please. This
fixes a puzzling test failure which only arises if the two tests
(PrioritizedExecutorsTests#testPriorityQueue and
PriorityTests#testCompareTo run in the same JVM, and run in the right
order).

Relates #19447
Jason Tedor 9 years ago
parent
commit
ac39e73183

+ 8 - 5
core/src/main/java/org/elasticsearch/common/Priority.java

@@ -23,9 +23,8 @@ import org.elasticsearch.common.io.stream.StreamOutput;
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
-import java.util.Locale;
 
 /**
  *
@@ -60,7 +59,8 @@ public final class Priority implements Comparable<Priority> {
     public static final Priority NORMAL = new Priority((byte) 2);
     public static final Priority LOW = new Priority((byte) 3);
     public static final Priority LANGUID = new Priority((byte) 4);
-    private static final List<Priority> values = Arrays.asList(IMMEDIATE, URGENT, HIGH, NORMAL, LOW, LANGUID);
+    private static final List<Priority> VALUES =
+            Collections.unmodifiableList(Arrays.asList(IMMEDIATE, URGENT, HIGH, NORMAL, LOW, LANGUID));
 
     private final byte value;
 
@@ -69,10 +69,13 @@ public final class Priority implements Comparable<Priority> {
     }
 
     /**
-     * @return a list of all available priorities, sorted from the highest to the lowest.
+     * All priorities, sorted from highest priority to lowest priority. The returned list is
+     * unmodifiable.
+     *
+     * @return an unmodifiable list of priorities, sorted from highest priority to lowest priority.
      */
     public static List<Priority> values() {
-        return values;
+        return VALUES;
     }
 
     @Override

+ 1 - 1
core/src/test/java/org/elasticsearch/common/util/concurrent/PrioritizedExecutorsTests.java

@@ -50,7 +50,7 @@ public class PrioritizedExecutorsTests extends ESTestCase {
 
     public void testPriorityQueue() throws Exception {
         PriorityBlockingQueue<Priority> queue = new PriorityBlockingQueue<>();
-        List<Priority> priorities = Priority.values();
+        List<Priority> priorities = new ArrayList<>(Priority.values());
         Collections.shuffle(priorities, random());
 
         for (Priority priority : priorities) {