Prevented empty filters from causing NPE

Closes #3724
This commit is contained in:
Luca Cavanna 2013-09-18 20:28:08 +02:00
parent 4958a6805f
commit cf0c360f86
9 changed files with 83 additions and 8 deletions

View File

@ -33,6 +33,7 @@ import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.AbstractIndexComponent;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.query.IndexQueryParserService;
import org.elasticsearch.index.query.ParsedFilter;
import org.elasticsearch.index.settings.IndexSettings;
import org.elasticsearch.indices.AliasFilterParsingException;
import org.elasticsearch.indices.InvalidAliasNameException;
@ -134,7 +135,8 @@ public class IndexAliasesService extends AbstractIndexComponent implements Itera
byte[] filterSource = filter.uncompressed();
XContentParser parser = XContentFactory.xContent(filterSource).createParser(filterSource);
try {
return indexQueryParser.parseInnerFilter(parser).filter();
ParsedFilter parsedFilter = indexQueryParser.parseInnerFilter(parser);
return parsedFilter == null ? null : parsedFilter.filter();
} finally {
parser.close();
}

View File

@ -13,6 +13,8 @@ public class ParsedFilter {
private final ImmutableMap<String, Filter> namedFilters;
public ParsedFilter(Filter filter, ImmutableMap<String, Filter> namedFilters) {
assert filter != null;
assert namedFilters != null;
this.filter = filter;
this.namedFilters = namedFilters;
}

View File

@ -23,6 +23,7 @@ import org.apache.lucene.search.Filter;
import org.elasticsearch.ElasticSearchIllegalArgumentException;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.ParsedFilter;
import org.elasticsearch.search.SearchParseElement;
import org.elasticsearch.search.SearchParseException;
import org.elasticsearch.search.facet.nested.NestedFacetExecutor;
@ -83,7 +84,8 @@ public class FacetParseElement implements SearchParseElement {
fieldName = parser.currentName();
} else if (token == XContentParser.Token.START_OBJECT) {
if ("facet_filter".equals(fieldName) || "facetFilter".equals(fieldName)) {
filter = context.queryParserService().parseInnerFilter(parser).filter();
ParsedFilter parsedFilter = context.queryParserService().parseInnerFilter(parser);
filter = parsedFilter == null ? null : parsedFilter.filter();
} else {
FacetParser facetParser = facetParsers.parser(fieldName);
if (facetParser == null) {

View File

@ -19,11 +19,11 @@
package org.elasticsearch.search.facet.filter;
import org.apache.lucene.search.Filter;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.ParsedFilter;
import org.elasticsearch.search.facet.FacetExecutor;
import org.elasticsearch.search.facet.FacetParser;
import org.elasticsearch.search.internal.SearchContext;
@ -58,7 +58,10 @@ public class FilterFacetParser extends AbstractComponent implements FacetParser
@Override
public FacetExecutor parse(String facetName, XContentParser parser, SearchContext context) throws IOException {
Filter facetFilter = context.queryParserService().parseInnerFilter(parser).filter();
return new FilterFacetExecutor(facetFilter);
ParsedFilter parsedFilter = context.queryParserService().parseInnerFilter(parser);
if (parsedFilter == null) {
parsedFilter = ParsedFilter.EMPTY;
}
return new FilterFacetExecutor(parsedFilter.filter());
}
}

View File

@ -34,6 +34,7 @@ import org.elasticsearch.index.fielddata.fieldcomparator.SortMode;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.ObjectMappers;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.query.ParsedFilter;
import org.elasticsearch.index.search.nested.NestedFieldComparatorSource;
import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
import org.elasticsearch.search.internal.SearchContext;
@ -73,7 +74,8 @@ public class GeoDistanceSortParser implements SortParser {
} else if (token == XContentParser.Token.START_OBJECT) {
// the json in the format of -> field : { lat : 30, lon : 12 }
if ("nested_filter".equals(currentName) || "nestedFilter".equals(currentName)) {
nestedFilter = context.queryParserService().parseInnerFilter(parser).filter();
ParsedFilter parsedFilter = context.queryParserService().parseInnerFilter(parser);
nestedFilter = parsedFilter == null ? null : parsedFilter.filter();
} else {
fieldName = currentName;
GeoPoint.parse(parser, point);

View File

@ -29,6 +29,7 @@ import org.elasticsearch.index.fielddata.fieldcomparator.SortMode;
import org.elasticsearch.index.fielddata.fieldcomparator.StringScriptDataComparator;
import org.elasticsearch.index.mapper.ObjectMappers;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.query.ParsedFilter;
import org.elasticsearch.index.search.nested.NestedFieldComparatorSource;
import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
import org.elasticsearch.script.SearchScript;
@ -67,7 +68,8 @@ public class ScriptSortParser implements SortParser {
if ("params".equals(currentName)) {
params = parser.map();
} else if ("nested_filter".equals(currentName) || "nestedFilter".equals(currentName)) {
nestedFilter = context.queryParserService().parseInnerFilter(parser).filter();
ParsedFilter parsedFilter = context.queryParserService().parseInnerFilter(parser);
nestedFilter = parsedFilter == null ? null : parsedFilter.filter();
}
} else if (token.isValue()) {
if ("reverse".equals(currentName)) {

View File

@ -33,6 +33,7 @@ import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.ObjectMappers;
import org.elasticsearch.index.mapper.core.NumberFieldMapper;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.query.ParsedFilter;
import org.elasticsearch.index.search.nested.NestedFieldComparatorSource;
import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
import org.elasticsearch.search.SearchParseElement;
@ -161,7 +162,8 @@ public class SortParseElement implements SearchParseElement {
}
} else if (token == XContentParser.Token.START_OBJECT) {
if ("nested_filter".equals(innerJsonName) || "nestedFilter".equals(innerJsonName)) {
nestedFilter = context.queryParserService().parseInnerFilter(parser).filter();
ParsedFilter parsedFilter = context.queryParserService().parseInnerFilter(parser);
nestedFilter = parsedFilter == null ? null : parsedFilter.filter();
} else {
throw new ElasticSearchIllegalArgumentException("sort option [" + innerJsonName + "] not supported");
}

View File

@ -140,6 +140,18 @@ public class IndexAliasesTests extends AbstractSharedClusterTest {
}
@Test
public void testEmptyFilter() throws Exception {
logger.info("--> creating index [test]");
createIndex("test");
ensureGreen();
logger.info("--> aliasing index [test] with [alias1] and empty filter");
IndicesAliasesResponse indicesAliasesResponse = client().admin().indices().prepareAliases().addAlias("test", "alias1", "{}").get();
//just checking that the empty doesn't lead to issues
assertThat(indicesAliasesResponse.isAcknowledged(), equalTo(true));
}
@Test
public void testSearchingFilteringAliasesSingleIndex() throws Exception {
// delete all indices

View File

@ -28,6 +28,7 @@ import org.elasticsearch.action.search.SearchType;
import org.elasticsearch.action.search.ShardSearchFailure;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentFactory;
@ -56,6 +57,7 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.FilterBuilders.termFilter;
import static org.elasticsearch.index.query.QueryBuilders.*;
import static org.elasticsearch.search.facet.FacetBuilders.*;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.hamcrest.Matchers.*;
/**
@ -80,6 +82,52 @@ public class SimpleFacetsTests extends AbstractSharedClusterTest {
return 5;
}
@Test
public void testSimpleFacetEmptyFacetFilter() throws Exception {
createIndex("test");
ensureGreen();
client().prepareIndex("test", "type1").setSource(jsonBuilder().startObject()
.field("tag", "green")
.endObject()).execute().actionGet();
refresh();
SearchResponse searchResponse = client().prepareSearch()
.setSearchType(SearchType.COUNT)
.setFacets(new BytesArray(
"{\"facet1\":{\"filter\":{ }}}").array())
.get();
assertHitCount(searchResponse, 1l);
assertThat(searchResponse.getHits().hits().length, equalTo(0));
FilterFacet facet = searchResponse.getFacets().facet("facet1");
assertThat(facet.getName(), equalTo("facet1"));
assertThat(facet.getCount(), equalTo(0l));
}
@Test
public void testSimpleFacetEmptyFilterFacet() throws Exception {
createIndex("test");
ensureGreen();
client().prepareIndex("test", "type1").setSource(jsonBuilder().startObject()
.field("tag", "green")
.endObject()).execute().actionGet();
refresh();
SearchResponse searchResponse = client().prepareSearch()
.setSearchType(SearchType.COUNT)
.setFacets(new BytesArray(
"{\"facet1\":{\"terms\":{\"field\":\"tag\"},\"facet_filter\":{ }}}").array())
.get();
assertHitCount(searchResponse, 1l);
assertThat(searchResponse.getHits().hits().length, equalTo(0));
TermsFacet facet = searchResponse.getFacets().facet("facet1");
assertThat(facet.getName(), equalTo("facet1"));
assertThat(facet.getEntries().size(), equalTo(1));
assertThat(facet.getEntries().get(0).getTerm().string(), equalTo("green"));
assertThat(facet.getEntries().get(0).getCount(), equalTo(1));
}
@Test
public void testBinaryFacet() throws Exception {
createIndex("test");