Browse Source

Make defensive copies of filters list internally

Christoph Büscher 9 years ago
parent
commit
f409d0b763

+ 9 - 2
core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorBuilder.java

@@ -59,8 +59,8 @@ public class FiltersAggregatorBuilder extends AggregatorBuilder<FiltersAggregato
     private FiltersAggregatorBuilder(String name, List<KeyedFilter> filters) {
         super(name, InternalFilters.TYPE);
         // internally we want to have a fixed order of filters, regardless of the order of the filters in the request
-        Collections.sort(filters, (KeyedFilter kf1, KeyedFilter kf2) -> kf1.key().compareTo(kf2.key()));
-        this.filters = filters;
+        this.filters = new ArrayList<>(filters);
+        Collections.sort(this.filters, (KeyedFilter kf1, KeyedFilter kf2) -> kf1.key().compareTo(kf2.key()));
         this.keyed = true;
     }
 
@@ -95,6 +95,13 @@ public class FiltersAggregatorBuilder extends AggregatorBuilder<FiltersAggregato
         return otherBucket;
     }
 
+    /**
+     * Get the filters. This will be an unmodifiable list
+     */
+    public List<KeyedFilter> filters() {
+        return Collections.unmodifiableList(this.filters);
+    }
+
     /**
      * Set the key to use for the bucket for documents not matching any
      * filter.

+ 2 - 4
core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java

@@ -490,9 +490,7 @@ public class FiltersIT extends ESIntegTestCase {
     }
 
     private static KeyedFilter[] randomOrder(KeyedFilter... filters) {
-        List<KeyedFilter> asList = Arrays.asList(filters);
-        Collections.shuffle(asList);
-        return asList.toArray(new KeyedFilter[filters.length]);
+        Collections.shuffle(Arrays.asList(filters), random());
+        return filters;
     }
-
 }

+ 17 - 0
core/src/test/java/org/elasticsearch/search/aggregations/metrics/FiltersTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.aggregations.metrics;
 
+import org.elasticsearch.index.query.MatchNoneQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.search.aggregations.BaseAggregationTestCase;
@@ -61,4 +62,20 @@ public class FiltersTests extends BaseAggregationTestCase<FiltersAggregatorBuild
         return factory;
     }
 
+    /**
+     * Test that when passing in keyed filters as list or array, the list stored internally is sorted by key
+     * Also check the list passed in is not modified by this but rather copied
+     */
+    public void testFiltersSortedByKey() {
+        KeyedFilter[] original = new KeyedFilter[]{new KeyedFilter("bbb", new MatchNoneQueryBuilder()),
+                new KeyedFilter("aaa", new MatchNoneQueryBuilder())};
+        FiltersAggregatorBuilder builder;
+        builder = new FiltersAggregatorBuilder("my-agg", original);
+        assertEquals("aaa", builder.filters().get(0).key());
+        assertEquals("bbb", builder.filters().get(1).key());
+        // original should be unchanged
+        assertEquals("bbb", original[0].key());
+        assertEquals("aaa", original[1].key());
+    }
+
 }