diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index ad3051ee785..8ff0930e9d1 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -3089,7 +3089,7 @@ - + diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java index baeaea434bd..482bcb3d009 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java @@ -35,13 +35,13 @@ import java.util.Map; public class FilterAggregatorFactory extends AggregatorFactory { - private final Weight weight; + final Weight weight; public FilterAggregatorFactory(String name, QueryBuilder filterBuilder, SearchContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, context, parent, subFactoriesBuilder, metaData); IndexSearcher contextSearcher = context.searcher(); - Query filter = filterBuilder.toQuery(context.getQueryShardContext()); + Query filter = filterBuilder.toFilter(context.getQueryShardContext()); weight = contextSearcher.createNormalizedWeight(filter, false); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorFactory.java index 2825c49b8b1..ded828d7623 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorFactory.java @@ -36,7 +36,7 @@ import java.util.Map; public class FiltersAggregatorFactory extends AggregatorFactory { private final String[] keys; - private final Weight[] weights; + final Weight[] weights; private final boolean keyed; private final boolean otherBucket; private final String otherBucketKey; diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java index 71e13d49a30..c27cabf78b2 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java @@ -65,7 +65,7 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac private MappedFieldType fieldType; private FilterableTermsEnum termsEnum; private int numberOfAggregatorsCreated; - private final Query filter; + final Query filter; private final int supersetNumDocs; private final TermsAggregator.BucketCountThresholds bucketCountThresholds; private final SignificanceHeuristic significanceHeuristic; @@ -79,7 +79,7 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac this.executionHint = executionHint; this.filter = filterBuilder == null ? null - : filterBuilder.toQuery(context.getQueryShardContext()); + : filterBuilder.toFilter(context.getQueryShardContext()); IndexSearcher searcher = context.searcher(); this.supersetNumDocs = filter == null // Important - need to use the doc count that includes deleted docs diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java b/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java index a0f7bbfcba4..9ebfb623b64 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java @@ -51,6 +51,7 @@ import org.elasticsearch.index.query.support.NestedScope; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache; +import org.elasticsearch.mock.orig.Mockito; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.subphase.DocValueFieldsFetchSubPhase; import org.elasticsearch.search.fetch.subphase.FetchSourceSubPhase; @@ -58,6 +59,7 @@ import org.elasticsearch.search.internal.ContextIndexSearcher; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.search.lookup.SearchLookup; import org.elasticsearch.test.ESTestCase; +import org.mockito.Matchers; import java.io.IOException; import java.util.ArrayList; @@ -80,9 +82,10 @@ public abstract class AggregatorTestCase extends ESTestCase { private static final String NESTEDFIELD_PREFIX = "nested_"; private List releasables = new ArrayList<>(); - protected A createAggregator(B aggregationBuilder, - IndexSearcher indexSearcher, - MappedFieldType... fieldTypes) throws IOException { + /** Create a factory for the given aggregation builder. */ + protected AggregatorFactory createAggregatorFactory(AggregationBuilder aggregationBuilder, + IndexSearcher indexSearcher, + MappedFieldType... fieldTypes) throws IOException { IndexSettings indexSettings = createIndexSettings(); SearchContext searchContext = createSearchContext(indexSearcher, indexSettings); CircuitBreakerService circuitBreakerService = new NoneCircuitBreakerService(); @@ -102,8 +105,14 @@ public abstract class AggregatorTestCase extends ESTestCase { QueryShardContext queryShardContext = queryShardContextMock(fieldTypes, indexSettings, circuitBreakerService); when(searchContext.getQueryShardContext()).thenReturn(queryShardContext); + return aggregationBuilder.build(searchContext, null); + } + + protected A createAggregator(AggregationBuilder aggregationBuilder, + IndexSearcher indexSearcher, + MappedFieldType... fieldTypes) throws IOException { @SuppressWarnings("unchecked") - A aggregator = (A) aggregationBuilder.build(searchContext, null).create(null, true); + A aggregator = (A) createAggregatorFactory(aggregationBuilder, indexSearcher, fieldTypes).create(null, true); return aggregator; } @@ -176,6 +185,8 @@ public abstract class AggregatorTestCase extends ESTestCase { new IndexFieldDataCache.None(), circuitBreakerService, mock(MapperService.class))); } NestedScope nestedScope = new NestedScope(); + when(queryShardContext.isFilter()).thenCallRealMethod(); + Mockito.doCallRealMethod().when(queryShardContext).setIsFilter(Matchers.anyBoolean()); when(queryShardContext.nestedScope()).thenReturn(nestedScope); return queryShardContext; } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterAggregatorTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java similarity index 75% rename from core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterAggregatorTests.java rename to core/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java index 54fd71d3e01..fb615e66dfb 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterAggregatorTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java @@ -16,26 +16,34 @@ * specific language governing permissions and limitations * under the License. */ -package org.elasticsearch.search.aggregations.bucket; +package org.elasticsearch.search.aggregations.bucket.filter; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.RandomIndexWriter; +import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.Query; import org.apache.lucene.store.Directory; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregatorFactory; import org.elasticsearch.search.aggregations.bucket.filter.InternalFilter; +import org.hamcrest.Matchers; import org.junit.Before; +import java.io.IOException; + public class FilterAggregatorTests extends AggregatorTestCase { private MappedFieldType fieldType; @@ -102,4 +110,22 @@ public class FilterAggregatorTests extends AggregatorTestCase { indexReader.close(); directory.close(); } + + public void testParsedAsFilter() throws IOException { + IndexReader indexReader = new MultiReader(); + IndexSearcher indexSearcher = newSearcher(indexReader); + QueryBuilder filter = QueryBuilders.boolQuery() + .must(QueryBuilders.termQuery("field", "foo")) + .should(QueryBuilders.termQuery("field", "bar")); + FilterAggregationBuilder builder = new FilterAggregationBuilder("test", filter); + AggregatorFactory factory = createAggregatorFactory(builder, indexSearcher, fieldType); + assertThat(factory, Matchers.instanceOf(FilterAggregatorFactory.class)); + FilterAggregatorFactory filterFactory = (FilterAggregatorFactory) factory; + Query parsedQuery = filterFactory.weight.getQuery(); + assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class)); + assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size()); + // means the bool query has been parsed as a filter, if it was a query minShouldMatch would + // be 0 + assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch()); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersAggregatorTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorTests.java similarity index 86% rename from core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersAggregatorTests.java rename to core/src/test/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorTests.java index 7ade659a122..4597eabb777 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersAggregatorTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorTests.java @@ -16,27 +16,34 @@ * specific language governing permissions and limitations * under the License. */ -package org.elasticsearch.search.aggregations.bucket; +package org.elasticsearch.search.aggregations.bucket.filters; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.RandomIndexWriter; +import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.Query; import org.apache.lucene.store.Directory; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregator; +import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregatorFactory; import org.elasticsearch.search.aggregations.bucket.filters.InternalFilters; +import org.hamcrest.Matchers; import org.junit.Before; +import java.io.IOException; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -200,4 +207,22 @@ public class FiltersAggregatorTests extends AggregatorTestCase { indexReader.close(); directory.close(); } + + public void testParsedAsFilter() throws IOException { + IndexReader indexReader = new MultiReader(); + IndexSearcher indexSearcher = newSearcher(indexReader); + QueryBuilder filter = QueryBuilders.boolQuery() + .must(QueryBuilders.termQuery("field", "foo")) + .should(QueryBuilders.termQuery("field", "bar")); + FiltersAggregationBuilder builder = new FiltersAggregationBuilder("test", filter); + AggregatorFactory factory = createAggregatorFactory(builder, indexSearcher, fieldType); + assertThat(factory, Matchers.instanceOf(FiltersAggregatorFactory.class)); + FiltersAggregatorFactory filtersFactory = (FiltersAggregatorFactory) factory; + Query parsedQuery = filtersFactory.weights[0].getQuery(); + assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class)); + assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size()); + // means the bool query has been parsed as a filter, if it was a query minShouldMatch would + // be 0 + assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch()); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java new file mode 100644 index 00000000000..e2625039df5 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java @@ -0,0 +1,75 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.aggregations.bucket.significant; + +import org.apache.lucene.index.IndexOptions; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.MultiReader; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.Query; +import org.elasticsearch.index.mapper.KeywordFieldMapper; +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.search.aggregations.AggregatorFactory; +import org.elasticsearch.search.aggregations.AggregatorTestCase; +import org.elasticsearch.search.aggregations.support.ValueType; +import org.hamcrest.Matchers; +import org.junit.Before; + +import java.io.IOException; + +public class SignificantTermsAggregatorTests extends AggregatorTestCase { + + private MappedFieldType fieldType; + + @Before + public void setUpTest() throws Exception { + super.setUp(); + fieldType = new KeywordFieldMapper.KeywordFieldType(); + fieldType.setHasDocValues(true); + fieldType.setIndexOptions(IndexOptions.DOCS); + fieldType.setName("field"); + } + + public void testParsedAsFilter() throws IOException { + IndexReader indexReader = new MultiReader(); + IndexSearcher indexSearcher = newSearcher(indexReader); + QueryBuilder filter = QueryBuilders.boolQuery() + .must(QueryBuilders.termQuery("field", "foo")) + .should(QueryBuilders.termQuery("field", "bar")); + SignificantTermsAggregationBuilder builder = new SignificantTermsAggregationBuilder( + "test", ValueType.STRING) + .field("field") + .backgroundFilter(filter); + AggregatorFactory factory = createAggregatorFactory(builder, indexSearcher, fieldType); + assertThat(factory, Matchers.instanceOf(SignificantTermsAggregatorFactory.class)); + SignificantTermsAggregatorFactory sigTermsFactory = + (SignificantTermsAggregatorFactory) factory; + Query parsedQuery = sigTermsFactory.filter; + assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class)); + assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size()); + // means the bool query has been parsed as a filter, if it was a query minShouldMatch would + // be 0 + assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch()); + } + +}