فهرست منبع

Parser throws NullPointerException when Filter aggregation clause is empty.
Added Junit test that recreates the error and fixed FilterParser to default to using a MatchAllDocsFilter if the requested filter clause is left empty.
Also added fix and test for the Filters (with an "s") aggregation.

Closes #8438

markharwood 11 سال پیش
والد
کامیت
0c94314996

+ 3 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterParser.java

@@ -18,6 +18,7 @@
  */
 package org.elasticsearch.search.aggregations.bucket.filter;
 
+import org.elasticsearch.common.lucene.search.MatchAllDocsFilter;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.ParsedFilter;
 import org.elasticsearch.search.aggregations.Aggregator;
@@ -39,7 +40,8 @@ public class FilterParser implements Aggregator.Parser {
     @Override
     public AggregatorFactory parse(String aggregationName, XContentParser parser, SearchContext context) throws IOException {
         ParsedFilter filter = context.queryParserService().parseInnerFilter(parser);
-        return new FilterAggregator.Factory(aggregationName, filter.filter());
+
+        return new FilterAggregator.Factory(aggregationName, filter == null ? new MatchAllDocsFilter() : filter.filter());
     }
 
 }

+ 4 - 2
src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersParser.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.aggregations.bucket.filters;
 
+import org.elasticsearch.common.lucene.search.MatchAllDocsFilter;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.ParsedFilter;
 import org.elasticsearch.search.SearchParseException;
@@ -60,7 +61,7 @@ public class FiltersParser implements Aggregator.Parser {
                             key = parser.currentName();
                         } else {
                             ParsedFilter filter = context.queryParserService().parseInnerFilter(parser);
-                            filters.add(new FiltersAggregator.KeyedFilter(key, filter.filter()));
+                            filters.add(new FiltersAggregator.KeyedFilter(key, filter == null ? new MatchAllDocsFilter() : filter.filter()));
                         }
                     }
                 } else {
@@ -72,7 +73,8 @@ public class FiltersParser implements Aggregator.Parser {
                     int idx = 0;
                     while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
                         ParsedFilter filter = context.queryParserService().parseInnerFilter(parser);
-                        filters.add(new FiltersAggregator.KeyedFilter(String.valueOf(idx), filter.filter()));
+                        filters.add(new FiltersAggregator.KeyedFilter(String.valueOf(idx), filter == null ? new MatchAllDocsFilter()
+                                : filter.filter()));
                         idx++;
                     }
                 } else {

+ 19 - 1
src/test/java/org/elasticsearch/search/aggregations/bucket/FilterTests.java

@@ -22,6 +22,8 @@ import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.index.query.AndFilterBuilder;
+import org.elasticsearch.index.query.FilterBuilder;
 import org.elasticsearch.search.aggregations.bucket.filter.Filter;
 import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
 import org.elasticsearch.search.aggregations.metrics.avg.Avg;
@@ -36,7 +38,9 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.FilterBuilders.matchAllFilter;
 import static org.elasticsearch.index.query.FilterBuilders.termFilter;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
-import static org.elasticsearch.search.aggregations.AggregationBuilders.*;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.avg;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.filter;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
@@ -103,6 +107,20 @@ public class FilterTests extends ElasticsearchIntegrationTest {
         assertThat(filter.getDocCount(), equalTo((long) numTag1Docs));
     }
 
+    // See NullPointer issue when filters are empty:
+    // https://github.com/elasticsearch/elasticsearch/issues/8438
+    @Test
+    public void emptyFilterDeclarations() throws Exception {
+        FilterBuilder emptyFilter = new AndFilterBuilder();
+        SearchResponse response = client().prepareSearch("idx").addAggregation(filter("tag1").filter(emptyFilter)).execute().actionGet();
+
+        assertSearchResponse(response);
+
+        Filter filter = response.getAggregations().get("tag1");
+        assertThat(filter, notNullValue());
+        assertThat(filter.getDocCount(), equalTo((long) numDocs));
+    }
+
     @Test
     public void withSubAggregation() throws Exception {
         SearchResponse response = client().prepareSearch("idx")

+ 26 - 1
src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersTests.java

@@ -23,6 +23,8 @@ import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.index.query.AndFilterBuilder;
+import org.elasticsearch.index.query.FilterBuilder;
 import org.elasticsearch.search.aggregations.bucket.filters.Filters;
 import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
 import org.elasticsearch.search.aggregations.metrics.avg.Avg;
@@ -39,7 +41,9 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.FilterBuilders.matchAllFilter;
 import static org.elasticsearch.index.query.FilterBuilders.termFilter;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
-import static org.elasticsearch.search.aggregations.AggregationBuilders.*;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.avg;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.filters;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
@@ -122,6 +126,27 @@ public class FiltersTests extends ElasticsearchIntegrationTest {
         assertThat(bucket.getDocCount(), equalTo((long) numTag2Docs));
     }
 
+    // See NullPointer issue when filters are empty:
+    // https://github.com/elasticsearch/elasticsearch/issues/8438
+    @Test
+    public void emptyFilterDeclarations() throws Exception {
+        FilterBuilder emptyFilter = new AndFilterBuilder();
+        SearchResponse response = client().prepareSearch("idx")
+                .addAggregation(filters("tags").filter("all", emptyFilter).filter("tag1", termFilter("tag", "tag1"))).execute()
+                .actionGet();
+
+        assertSearchResponse(response);
+
+        Filters filters = response.getAggregations().get("tags");
+        assertThat(filters, notNullValue());
+        Filters.Bucket allBucket = filters.getBucketByKey("all");
+        assertThat(allBucket.getDocCount(), equalTo((long) numDocs));
+
+        Filters.Bucket bucket = filters.getBucketByKey("tag1");
+        assertThat(bucket, Matchers.notNullValue());
+        assertThat(bucket.getDocCount(), equalTo((long) numTag1Docs));
+    }
+
     @Test
     public void withSubAggregation() throws Exception {
         SearchResponse response = client().prepareSearch("idx")