From f6ca11db30708d308363b17c5dd37780f7a51c3f Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 27 Jul 2015 11:25:08 +0200 Subject: [PATCH] [TEST] remove createExpectedQuery from BaseQueryTestCase We currently test the toQuery method by comparing its return value with the output of createExpectedQuery. This seemed at first like a good deep test for the lucene generated queries, but ends up causing a lot of code duplication between tests and prod code, which smells like this test is not that useful after all. This commit removes the requirement for createExpectedQuery, we test a bit less the lucene generated queries, but we do still have a testToQuery method in BaseQueryTestCase. This test acts as smoke test to verify that there are no issues with our toQuery method implementation for each query: it verifies that two equal queries should result in the same lucene query. It also verifies that changing the boost value of a query affects the result of the toQuery. Part of the previous createExpectedQuery can be moved to assertLuceneQuery using normal assertions, we do it when it makes sense only though. This commit also adds support for boost and queryName to SpanMultiTermQueryParser and SpanMultiTermQueryBuilder, plus it removes no-op setters for queryName and boost from QueryFilterBuilder. Instead we simply declare in our test infra that query filter doesn't support boost and queryName and we disable tests aroudn these fields in that specific case. Boost and queryName support is also moved down to the QueryBuilder interface. Closes #12473 --- .../index/query/AbstractQueryBuilder.java | 8 +- .../index/query/EmptyQueryBuilder.java | 22 ++++ .../index/query/FuzzyQueryBuilder.java | 10 -- .../index/query/LimitQueryBuilder.java | 4 + .../index/query/QueryBuilder.java | 21 ++++ .../index/query/QueryFilterBuilder.java | 12 -- .../query/SpanMultiTermQueryBuilder.java | 13 +- .../index/query/SpanMultiTermQueryParser.java | 45 ++++--- .../index/query/AndQueryBuilderTest.java | 54 +++++--- .../index/query/BaseQueryTestCase.java | 101 +++++++++------ .../index/query/BaseTermQueryTestCase.java | 29 +---- .../index/query/BoolQueryBuilderTest.java | 70 +++++++---- .../index/query/BoostingQueryBuilderTest.java | 10 +- .../query/CommonTermsQueryBuilderTest.java | 34 ++--- .../query/ConstantScoreQueryBuilderTest.java | 25 ++-- .../index/query/DisMaxQueryBuilderTest.java | 31 +++-- .../index/query/ExistsQueryBuilderTest.java | 73 +++++------ .../index/query/FQueryFilterBuilderTest.java | 25 ++-- .../FieldMaskingSpanQueryBuilderTest.java | 29 +++-- .../index/query/FilteredQueryBuilderTest.java | 46 ++++--- .../index/query/IdsQueryBuilderTest.java | 39 +++--- .../index/query/LimitQueryBuilderTest.java | 17 +-- .../index/query/MatchAllQueryBuilderTest.java | 13 +- .../index/query/MissingQueryBuilderTest.java | 119 +----------------- .../index/query/NotQueryBuilderTest.java | 32 +++-- .../index/query/OrQueryBuilderTest.java | 39 +++++- .../index/query/PrefixQueryBuilderTest.java | 32 +---- .../index/query/QueryFilterBuilderTest.java | 51 ++++++-- .../index/query/RangeQueryBuilderTest.java | 34 ++--- .../index/query/RegexpQueryBuilderTest.java | 28 +---- .../index/query/ScriptQueryBuilderTest.java | 5 +- .../query/SimpleQueryStringBuilderTest.java | 58 +-------- .../query/SpanContainingQueryBuilderTest.java | 15 ++- .../query/SpanFirstQueryBuilderTest.java | 13 +- .../query/SpanMultiTermQueryBuilderTest.java | 18 ++- .../index/query/SpanNearQueryBuilderTest.java | 31 ++--- .../index/query/SpanNotQueryBuilderTest.java | 17 +-- .../index/query/SpanOrQueryBuilderTest.java | 28 +++-- .../index/query/SpanTermQueryBuilderTest.java | 22 +++- .../query/SpanWithinQueryBuilderTest.java | 15 ++- .../index/query/TermQueryBuilderTest.java | 22 +++- .../index/query/TypeQueryBuilderTest.java | 22 ++-- .../index/query/WildcardQueryBuilderTest.java | 29 +---- 43 files changed, 664 insertions(+), 697 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index 738174b836b..e674a8f1cd1 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -95,7 +95,8 @@ public abstract class AbstractQueryBuilder exte * Returns the query name for the query. */ @SuppressWarnings("unchecked") - public QB queryName(String queryName) { + @Override + public final QB queryName(String queryName) { this.queryName = queryName; return (QB) this; } @@ -103,6 +104,7 @@ public abstract class AbstractQueryBuilder exte /** * Sets the query name for the query. */ + @Override public final String queryName() { return queryName; } @@ -110,6 +112,7 @@ public abstract class AbstractQueryBuilder exte /** * Returns the boost for this query. */ + @Override public final float boost() { return this.boost; } @@ -119,7 +122,8 @@ public abstract class AbstractQueryBuilder exte * weightings) have their score multiplied by the boost provided. */ @SuppressWarnings("unchecked") - public QB boost(float boost) { + @Override + public final QB boost(float boost) { this.boost = boost; return (QB) this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java index 971f0719f19..64b84bbe1b8 100644 --- a/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java @@ -81,4 +81,26 @@ public class EmptyQueryBuilder extends ToXContentToBytes implements QueryBuilder public EmptyQueryBuilder readFrom(StreamInput in) throws IOException { return EmptyQueryBuilder.PROTOTYPE; } + + @Override + public EmptyQueryBuilder queryName(String queryName) { + //no-op + return this; + } + + @Override + public String queryName() { + return null; + } + + @Override + public float boost() { + return -1; + } + + @Override + public EmptyQueryBuilder boost(float boost) { + //no-op + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java index 2c7bf0ec9da..2984d829d5b 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java @@ -120,16 +120,6 @@ public class FuzzyQueryBuilder extends AbstractQueryBuilder i this(name, (Object) value); } - /** - * Sets the boost for this query. Documents matching this query will (in addition to the normal - * weightings) have their score multiplied by the boost provided. - */ - @Override - public FuzzyQueryBuilder boost(float boost) { - this.boost = boost; - return this; - } - public FuzzyQueryBuilder fuzziness(Fuzziness fuzziness) { this.fuzziness = fuzziness; return this; diff --git a/core/src/main/java/org/elasticsearch/index/query/LimitQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/LimitQueryBuilder.java index 97207b69a51..f3731509789 100644 --- a/core/src/main/java/org/elasticsearch/index/query/LimitQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/LimitQueryBuilder.java @@ -42,6 +42,10 @@ public class LimitQueryBuilder extends AbstractQueryBuilder { this.limit = limit; } + public int limit() { + return limit; + } + @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java index 567b3169a5c..e13337d7107 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java @@ -56,4 +56,25 @@ public interface QueryBuilder extends NamedWriteable return this.queryBuilder; } - @Override - public QueryFilterBuilder boost(float boost) { - //no-op: QueryFilterParser doesn't support boost, we should be consistent and ignore it here too. - return this; - } - - @Override - public QueryFilterBuilder queryName(String queryName) { - //no-op: QueryFilterParser doesn't support _name, we should be consistent and ignore it here too. - return this; - } - @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.field(NAME); diff --git a/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java index 60c09f1ba73..697c7fe0bd6 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java @@ -52,6 +52,7 @@ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder { - @Override - protected Query doCreateExpectedQuery(AndQueryBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException { - if (queryBuilder.filters().isEmpty()) { - return null; - } - BooleanQuery query = new BooleanQuery(); - for (QueryBuilder subQuery : queryBuilder.filters()) { - Query innerQuery = subQuery.toQuery(context); - if (innerQuery != null) { - query.add(innerQuery, Occur.MUST); - } - } - if (query.clauses().isEmpty()) { - return null; - } - return query; - } - /** * @return a AndQueryBuilder with random limit between 0 and 20 */ @@ -62,6 +51,33 @@ public class AndQueryBuilderTest extends BaseQueryTestCase { return query; } + @Override + protected void doAssertLuceneQuery(AndQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + if (queryBuilder.filters().isEmpty()) { + assertThat(query, nullValue()); + } else { + List clauses = new ArrayList<>(); + for (QueryBuilder innerFilter : queryBuilder.filters()) { + Query clause = innerFilter.toQuery(context); + if (clause != null) { + clauses.add(clause); + } + } + if (clauses.isEmpty()) { + assertThat(query, nullValue()); + } else { + assertThat(query, instanceOf(BooleanQuery.class)); + BooleanQuery booleanQuery = (BooleanQuery) query; + assertThat(booleanQuery.clauses().size(), equalTo(clauses.size())); + Iterator queryIterator = clauses.iterator(); + for (BooleanClause booleanClause : booleanQuery) { + assertThat(booleanClause.getOccur(), equalTo(BooleanClause.Occur.MUST)); + assertThat(booleanClause.getQuery(), equalTo(queryIterator.next())); + } + } + } + } + /** * test corner case where no inner queries exist */ diff --git a/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java index 8d879ff0cc7..1e39eeeab78 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java @@ -25,7 +25,7 @@ import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.cluster.ClusterService; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.Injector; @@ -180,11 +180,13 @@ public abstract class BaseQueryTestCase> ext protected final QB createTestQueryBuilder() { QB query = doCreateTestQueryBuilder(); - if (randomBoolean()) { - query.boost(2.0f / randomIntBetween(1, 20)); - } - if (randomBoolean()) { - query.queryName(randomAsciiOfLengthBetween(1, 10)); + if (supportsBoostAndQueryName()) { + if (randomBoolean()) { + query.boost(2.0f / randomIntBetween(1, 20)); + } + if (randomBoolean()) { + query.queryName(randomAsciiOfLengthBetween(1, 10)); + } } return query; } @@ -219,48 +221,71 @@ public abstract class BaseQueryTestCase> ext */ @Test public void testToQuery() throws IOException { - QB testQuery = createTestQueryBuilder(); QueryParseContext context = createContext(); context.setAllowUnmappedFields(true); - Query expectedQuery = createExpectedQuery(testQuery, context); - Query actualQuery = testQuery.toQuery(context); - // expectedQuery can be null, e.g. in case of BoostingQueryBuilder - // with inner clause that returns null itself - if (expectedQuery == null) { - assertNull("Expected a null query, saw some object.", actualQuery); - } else { - assertThat(actualQuery, instanceOf(expectedQuery.getClass())); - assertThat(actualQuery, equalTo(expectedQuery)); - assertLuceneQuery(testQuery, actualQuery, context); + QB firstQuery = createTestQueryBuilder(); + Query firstLuceneQuery = firstQuery.toQuery(context); + assertLuceneQuery(firstQuery, firstLuceneQuery, context); + + try (BytesStreamOutput output = new BytesStreamOutput()) { + firstQuery.writeTo(output); + try (StreamInput in = new FilterStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) { + QueryBuilder prototype = queryParserService.queryParser(firstQuery.getName()).getBuilderPrototype(); + @SuppressWarnings("unchecked") + QB secondQuery = (QB)prototype.readFrom(in); + //query _name never should affect the result of toQuery, we randomly set it to make sure + if (randomBoolean()) { + secondQuery.queryName(secondQuery.queryName() == null ? randomAsciiOfLengthBetween(1, 30) : secondQuery.queryName() + randomAsciiOfLengthBetween(1, 10)); + } + Query secondLuceneQuery = secondQuery.toQuery(context); + assertLuceneQuery(secondQuery, secondLuceneQuery, context); + assertThat("two equivalent query builders lead to different lucene queries", secondLuceneQuery, equalTo(firstLuceneQuery)); + + //if the initial lucene query is null, changing its boost won't have any effect, we shouldn't test that + //otherwise makes sure that boost is taken into account in toQuery + if (firstLuceneQuery != null) { + secondQuery.boost(firstQuery.boost() + 1f + randomFloat()); + //some queries don't support boost, their setter is a no-op + if (supportsBoostAndQueryName()) { + Query thirdLuceneQuery = secondQuery.toQuery(context); + assertThat("modifying the boost doesn't affect the corresponding lucene query", firstLuceneQuery, not(equalTo(thirdLuceneQuery))); + } + } + } } } - protected final Query createExpectedQuery(QB queryBuilder, QueryParseContext context) throws IOException { - Query expectedQuery = doCreateExpectedQuery(queryBuilder, context); - if (expectedQuery != null) { - expectedQuery.setBoost(queryBuilder.boost()); - } - return expectedQuery; + /** + * Few queries allow you to set the boost and queryName but don't do anything with it. This method allows + * to disable boost and queryName related tests for those queries. + */ + protected boolean supportsBoostAndQueryName() { + return true; } /** - * Creates the expected lucene query given the current {@link QueryBuilder} and {@link QueryParseContext}. - * The returned query will be compared with the result of {@link QueryBuilder#toQuery(QueryParseContext)} to test its behaviour. + * Checks the result of {@link QueryBuilder#toQuery(QueryParseContext)} given the original {@link QueryBuilder} and {@link QueryParseContext}. + * Verifies that named queries and boost are properly handled and delegates to {@link #doAssertLuceneQuery(AbstractQueryBuilder, Query, QueryParseContext)} + * for query specific checks. */ - protected abstract Query doCreateExpectedQuery(QB queryBuilder, QueryParseContext context) throws IOException; - - /** - * Run after default equality comparison between lucene expected query and result of {@link QueryBuilder#toQuery(QueryParseContext)}. - * Can contain additional assertions that are query specific. Default implementation verifies that names queries are properly handled. - */ - protected final void assertLuceneQuery(QB queryBuilder, Query query, QueryParseContext context) { + protected final void assertLuceneQuery(QB queryBuilder, Query query, QueryParseContext context) throws IOException { if (queryBuilder.queryName() != null) { Query namedQuery = context.copyNamedQueries().get(queryBuilder.queryName()); assertThat(namedQuery, equalTo(query)); } + if (query != null) { + assertThat(query.getBoost(), equalTo(queryBuilder.boost())); + } + doAssertLuceneQuery(queryBuilder, query, context); } + /** + * Checks the result of {@link QueryBuilder#toQuery(QueryParseContext)} given the original {@link QueryBuilder} and {@link QueryParseContext}. + * Contains the query specific checks to be implemented by subclasses. + */ + protected abstract void doAssertLuceneQuery(QB queryBuilder, Query query, QueryParseContext context) throws IOException; + /** * Test serialization and deserialization of the test query. */ @@ -283,7 +308,9 @@ public abstract class BaseQueryTestCase> ext * @return a new {@link QueryParseContext} based on the base test index and queryParserService */ protected static QueryParseContext createContext() { - return new QueryParseContext(index, queryParserService); + QueryParseContext queryParseContext = new QueryParseContext(index, queryParserService); + queryParseContext.parseFieldMatcher(ParseFieldMatcher.EMPTY); + return queryParseContext; } protected static void assertQueryHeader(XContentParser parser, String expectedParserName) throws IOException { @@ -325,13 +352,13 @@ public abstract class BaseQueryTestCase> ext protected static String getRandomRewriteMethod() { String rewrite; if (randomBoolean()) { - rewrite = randomFrom(new ParseField[]{QueryParsers.CONSTANT_SCORE, + rewrite = randomFrom(QueryParsers.CONSTANT_SCORE, QueryParsers.SCORING_BOOLEAN, - QueryParsers.CONSTANT_SCORE_BOOLEAN}).getPreferredName(); + QueryParsers.CONSTANT_SCORE_BOOLEAN).getPreferredName(); } else { - rewrite = randomFrom(new ParseField[]{QueryParsers.TOP_TERMS, + rewrite = randomFrom(QueryParsers.TOP_TERMS, QueryParsers.TOP_TERMS_BOOST, - QueryParsers.TOP_TERMS_BLENDED_FREQS}).getPreferredName() + "1"; + QueryParsers.TOP_TERMS_BLENDED_FREQS).getPreferredName() + "1"; } return rewrite; } diff --git a/core/src/test/java/org/elasticsearch/index/query/BaseTermQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/BaseTermQueryTestCase.java index d8df0aba19f..6f4ced76188 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BaseTermQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/BaseTermQueryTestCase.java @@ -19,11 +19,6 @@ package org.elasticsearch.index.query; -import org.apache.lucene.index.Term; -import org.apache.lucene.search.Query; -import org.apache.lucene.util.BytesRef; -import org.elasticsearch.common.lucene.BytesRefs; -import org.elasticsearch.index.mapper.MappedFieldType; import org.junit.Ignore; import org.junit.Test; @@ -79,15 +74,14 @@ public abstract class BaseTermQueryTestCase> @Test public void testValidate() throws QueryParsingException { - - QB queryBuilder = createQueryBuilder("all", "good"); + QB queryBuilder = createQueryBuilder(randomAsciiOfLengthBetween(1, 30), randomAsciiOfLengthBetween(1, 30)); assertNull(queryBuilder.validate()); - queryBuilder = createQueryBuilder(null, "Term"); + queryBuilder = createQueryBuilder(null, randomAsciiOfLengthBetween(1, 30)); assertNotNull(queryBuilder.validate()); assertThat(queryBuilder.validate().validationErrors().size(), is(1)); - queryBuilder = createQueryBuilder("", "Term"); + queryBuilder = createQueryBuilder("", randomAsciiOfLengthBetween(1, 30)); assertNotNull(queryBuilder.validate()); assertThat(queryBuilder.validate().validationErrors().size(), is(1)); @@ -95,21 +89,4 @@ public abstract class BaseTermQueryTestCase> assertNotNull(queryBuilder.validate()); assertThat(queryBuilder.validate().validationErrors().size(), is(2)); } - - @Override - protected Query doCreateExpectedQuery(QB queryBuilder, QueryParseContext context) { - BytesRef value = null; - if (getCurrentTypes().length > 0) { - if (queryBuilder.fieldName().equals(BOOLEAN_FIELD_NAME) || queryBuilder.fieldName().equals(INT_FIELD_NAME) || queryBuilder.fieldName().equals(DOUBLE_FIELD_NAME)) { - MappedFieldType mapper = context.fieldMapper(queryBuilder.fieldName()); - value = mapper.indexedValueForSearch(queryBuilder.value); - } - } - if (value == null) { - value = BytesRefs.toBytesRef(queryBuilder.value); - } - return createLuceneTermQuery(new Term(queryBuilder.fieldName(), value)); - } - - protected abstract Query createLuceneTermQuery(Term term); } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java index de36d7e7a11..f9283f5f0d2 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java @@ -20,17 +20,18 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; -import org.elasticsearch.common.lucene.search.Queries; import org.junit.Test; import java.io.IOException; +import java.util.ArrayList; +import java.util.Iterator; import java.util.List; -import static org.elasticsearch.common.lucene.search.Queries.fixNegativeQueryIfNeeded; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; public class BoolQueryBuilderTest extends BaseQueryTestCase { @@ -66,34 +67,53 @@ public class BoolQueryBuilderTest extends BaseQueryTestCase { } @Override - protected Query doCreateExpectedQuery(BoolQueryBuilder queryBuilder, QueryParseContext context) throws IOException { + protected void doAssertLuceneQuery(BoolQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { if (!queryBuilder.hasClauses()) { - return new MatchAllDocsQuery(); - } + assertThat(query, instanceOf(MatchAllDocsQuery.class)); + } else { + List clauses = new ArrayList<>(); + clauses.addAll(getBooleanClauses(queryBuilder.must(), BooleanClause.Occur.MUST, context)); + clauses.addAll(getBooleanClauses(queryBuilder.mustNot(), BooleanClause.Occur.MUST_NOT, context)); + clauses.addAll(getBooleanClauses(queryBuilder.should(), BooleanClause.Occur.SHOULD, context)); + clauses.addAll(getBooleanClauses(queryBuilder.filter(), BooleanClause.Occur.FILTER, context)); - BooleanQuery boolQuery = new BooleanQuery(queryBuilder.disableCoord()); - addBooleanClauses(context, boolQuery, queryBuilder.must(), BooleanClause.Occur.MUST); - addBooleanClauses(context, boolQuery, queryBuilder.mustNot(), BooleanClause.Occur.MUST_NOT); - addBooleanClauses(context, boolQuery, queryBuilder.should(), BooleanClause.Occur.SHOULD); - addBooleanClauses(context, boolQuery, queryBuilder.filter(), BooleanClause.Occur.FILTER); - - if (boolQuery.clauses().isEmpty()) { - return new MatchAllDocsQuery(); - } - Queries.applyMinimumShouldMatch(boolQuery, queryBuilder.minimumNumberShouldMatch()); - return queryBuilder.adjustPureNegative() ? fixNegativeQueryIfNeeded(boolQuery) : boolQuery; - } - - private static void addBooleanClauses(QueryParseContext parseContext, BooleanQuery booleanQuery, List clauses, Occur occurs) - throws IOException { - for (QueryBuilder query : clauses) { - Query innerQuery = query.toQuery(parseContext); - if (innerQuery != null) { - booleanQuery.add(new BooleanClause(innerQuery, occurs)); + if (clauses.isEmpty()) { + assertThat(query, instanceOf(MatchAllDocsQuery.class)); + } else { + assertThat(query, instanceOf(BooleanQuery.class)); + BooleanQuery booleanQuery = (BooleanQuery) query; + if (queryBuilder.adjustPureNegative()) { + boolean isNegative = true; + for (BooleanClause clause : clauses) { + if (clause.isProhibited() == false) { + isNegative = false; + break; + } + } + if (isNegative) { + clauses.add(new BooleanClause(new MatchAllDocsQuery(), BooleanClause.Occur.MUST)); + } + } + assertThat(booleanQuery.clauses().size(), equalTo(clauses.size())); + Iterator clauseIterator = clauses.iterator(); + for (BooleanClause booleanClause : booleanQuery.getClauses()) { + assertThat(booleanClause, equalTo(clauseIterator.next())); + } } } } + private static List getBooleanClauses(List queryBuilders, BooleanClause.Occur occur, QueryParseContext parseContext) throws IOException { + List clauses = new ArrayList<>(); + for (QueryBuilder query : queryBuilders) { + Query innerQuery = query.toQuery(parseContext); + if (innerQuery != null) { + clauses.add(new BooleanClause(innerQuery, occur)); + } + } + return clauses; + } + @Test public void testValidate() { BoolQueryBuilder booleanQuery = new BoolQueryBuilder(); diff --git a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java index 7d22ea6e3f9..742e0a1f203 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java @@ -25,6 +25,9 @@ import org.junit.Test; import java.io.IOException; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.nullValue; + public class BoostingQueryBuilderTest extends BaseQueryTestCase { @Override @@ -35,13 +38,14 @@ public class BoostingQueryBuilderTest extends BaseQueryTestCase { @@ -77,30 +76,11 @@ public class CommonTermsQueryBuilderTest extends BaseQueryTestCase { +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.nullValue; - @Override - protected Query doCreateExpectedQuery(ConstantScoreQueryBuilder testBuilder, QueryParseContext context) throws QueryParsingException, IOException { - Query innerQuery = testBuilder.query().toQuery(context); - if (innerQuery != null) { - return new ConstantScoreQuery(innerQuery); - } - return null; - } +public class ConstantScoreQueryBuilderTest extends BaseQueryTestCase { /** * @return a {@link ConstantScoreQueryBuilder} with random boost between 0.1f and 2.0f @@ -46,6 +41,18 @@ public class ConstantScoreQueryBuilderTest extends BaseQueryTestCase { - @Override - protected Query doCreateExpectedQuery(DisMaxQueryBuilder testBuilder, QueryParseContext context) throws QueryParsingException, IOException { - Collection queries = AbstractQueryBuilder.toQueries(testBuilder.queries(), context); - if (queries.isEmpty()) { - return null; - } - return new DisjunctionMaxQuery(queries, testBuilder.tieBreaker()); - } - /** * @return a {@link DisMaxQueryBuilder} with random inner queries */ @@ -55,6 +51,23 @@ public class DisMaxQueryBuilderTest extends BaseQueryTestCase queries = AbstractQueryBuilder.toQueries(queryBuilder.queries(), context); + if (queries.isEmpty()) { + assertThat(query, nullValue()); + } else { + assertThat(query, instanceOf(DisjunctionMaxQuery.class)); + DisjunctionMaxQuery disjunctionMaxQuery = (DisjunctionMaxQuery) query; + assertThat(disjunctionMaxQuery.getTieBreakerMultiplier(), equalTo(queryBuilder.tieBreaker())); + assertThat(disjunctionMaxQuery.getDisjuncts().size(), equalTo(queries.size())); + Iterator queryIterator = queries.iterator(); + for (int i = 0; i < disjunctionMaxQuery.getDisjuncts().size(); i++) { + assertThat(disjunctionMaxQuery.getDisjuncts().get(i), equalTo(queryIterator.next())); + } + } + } + /** * test `null`return value for missing inner queries * @throws IOException diff --git a/core/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTest.java index d2ddfc29477..135573b527f 100644 --- a/core/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTest.java @@ -19,52 +19,21 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.*; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.Query; import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.common.lucene.search.Queries; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper; +import org.elasticsearch.index.mapper.object.ObjectMapper; import java.io.IOException; import java.util.Collection; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; + public class ExistsQueryBuilderTest extends BaseQueryTestCase { - private static Collection getFieldNamePattern(String fieldName, QueryParseContext context) { - if (getCurrentTypes().length > 0 && fieldName.equals(BaseQueryTestCase.OBJECT_FIELD_NAME)) { - // "object" field has two inner fields (age, price), so if query hits that field, we - // extend field name with wildcard to match both nested fields. This is similar to what - // is done internally in ExistsQueryBuilder.toQuery() - fieldName = fieldName + ".*"; - } - return context.simpleMatchToIndexNames(fieldName); - } - - @Override - protected Query doCreateExpectedQuery(ExistsQueryBuilder queryBuilder, QueryParseContext context) throws IOException { - final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType)context.mapperService().fullName(FieldNamesFieldMapper.NAME); - Collection fields = getFieldNamePattern(queryBuilder.name(), context); - - if (fields.isEmpty() || fieldNamesFieldType == null) { - return Queries.newMatchNoDocsQuery(); - } - - BooleanQuery boolFilter = new BooleanQuery(); - for (String field : fields) { - if (fieldNamesFieldType.isEnabled()) { - boolFilter.add(fieldNamesFieldType.termQuery(field, context), BooleanClause.Occur.SHOULD); - } else { - MappedFieldType fieldType = context.fieldMapper(field); - if (fieldType == null) { - boolFilter.add(new TermRangeQuery(field, null, null, true, true), BooleanClause.Occur.SHOULD); - } else { - boolFilter.add(fieldType.rangeQuery(null, null, true, true), BooleanClause.Occur.SHOULD); - } - } - } - return new ConstantScoreQuery(boolFilter); - } - @Override protected ExistsQueryBuilder doCreateTestQueryBuilder() { String fieldPattern; @@ -83,4 +52,30 @@ public class ExistsQueryBuilderTest extends BaseQueryTestCase fields = context.simpleMatchToIndexNames(fieldPattern); + if (getCurrentTypes().length == 0 || fields.size() == 0) { + assertThat(query, instanceOf(BooleanQuery.class)); + BooleanQuery booleanQuery = (BooleanQuery) query; + assertThat(booleanQuery.clauses().size(), equalTo(0)); + } else { + assertThat(query, instanceOf(ConstantScoreQuery.class)); + ConstantScoreQuery constantScoreQuery = (ConstantScoreQuery) query; + assertThat(constantScoreQuery.getQuery(), instanceOf(BooleanQuery.class)); + BooleanQuery booleanQuery = (BooleanQuery) constantScoreQuery.getQuery(); + assertThat(booleanQuery.clauses().size(), equalTo(fields.size())); + for (int i = 0; i < fields.size(); i++) { + BooleanClause booleanClause = booleanQuery.clauses().get(i); + assertThat(booleanClause.getOccur(), equalTo(BooleanClause.Occur.SHOULD)); + } + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/FQueryFilterBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/FQueryFilterBuilderTest.java index e9995559664..2a4a52986ab 100644 --- a/core/src/test/java/org/elasticsearch/index/query/FQueryFilterBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/FQueryFilterBuilderTest.java @@ -27,18 +27,13 @@ import org.junit.Test; import java.io.IOException; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.nullValue; + @SuppressWarnings("deprecation") public class FQueryFilterBuilderTest extends BaseQueryTestCase { - @Override - protected Query doCreateExpectedQuery(FQueryFilterBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException { - Query query = queryBuilder.innerQuery().toQuery(context); - if (query != null) { - return new ConstantScoreQuery(query); - } - return null; - } - /** * @return a FQueryFilterBuilder with random inner query */ @@ -48,6 +43,18 @@ public class FQueryFilterBuilderTest extends BaseQueryTestCase { - @Override - protected Query doCreateExpectedQuery(FieldMaskingSpanQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { - String fieldInQuery = testQueryBuilder.fieldName(); - MappedFieldType fieldType = context.fieldMapper(fieldInQuery); - if (fieldType != null) { - fieldInQuery = fieldType.names().indexName(); - } - SpanQuery innerQuery = (SpanQuery) testQueryBuilder.innerQuery().toQuery(context); - - return new FieldMaskingSpanQuery(innerQuery, fieldInQuery); - } - @Override protected FieldMaskingSpanQueryBuilder doCreateTestQueryBuilder() { String fieldName; @@ -53,6 +43,19 @@ public class FieldMaskingSpanQueryBuilderTest extends BaseQueryTestCase { @@ -33,30 +39,32 @@ public class FilteredQueryBuilderTest extends BaseQueryTestCase { @@ -50,26 +49,6 @@ public class IdsQueryBuilderTest extends BaseQueryTestCase { context.indexQueryParserService().queryParser("ids").fromXContent(context); } - @Override - protected Query doCreateExpectedQuery(IdsQueryBuilder queryBuilder, QueryParseContext context) throws IOException { - Query expectedQuery; - if (queryBuilder.ids().size() == 0) { - expectedQuery = Queries.newMatchNoDocsQuery(); - } else { - String[] typesForQuery; - if (queryBuilder.types() == null || queryBuilder.types().length == 0) { - Collection queryTypes = context.queryTypes(); - typesForQuery = queryTypes.toArray(new String[queryTypes.size()]); - } else if (queryBuilder.types().length == 1 && MetaData.ALL.equals(queryBuilder.types()[0])) { - typesForQuery = getCurrentTypes(); - } else { - typesForQuery = queryBuilder.types(); - } - expectedQuery = new TermsQuery(UidFieldMapper.NAME, Uid.createUidsForTypesAndIds(Sets.newHashSet(typesForQuery), queryBuilder.ids())); - } - return expectedQuery; - } - @Override protected IdsQueryBuilder doCreateTestQueryBuilder() { String[] types; @@ -105,4 +84,14 @@ public class IdsQueryBuilderTest extends BaseQueryTestCase { } return query; } + + @Override + protected void doAssertLuceneQuery(IdsQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + if (queryBuilder.ids().size() == 0) { + assertThat(query, instanceOf(BooleanQuery.class)); + assertThat(((BooleanQuery)query).clauses().size(), equalTo(0)); + } else { + assertThat(query, instanceOf(TermsQuery.class)); + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/LimitQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/LimitQueryBuilderTest.java index 06adde306e3..b5c795f1663 100644 --- a/core/src/test/java/org/elasticsearch/index/query/LimitQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/LimitQueryBuilderTest.java @@ -19,17 +19,15 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; -import org.elasticsearch.common.lucene.search.Queries; + +import java.io.IOException; + +import static org.hamcrest.CoreMatchers.instanceOf; public class LimitQueryBuilderTest extends BaseQueryTestCase { - @Override - protected Query doCreateExpectedQuery(LimitQueryBuilder queryBuilder, QueryParseContext context) { - // this filter is deprecated and parses to a filter that matches everything - return Queries.newMatchAllQuery(); - } - /** * @return a LimitQueryBuilder with random limit between 0 and 20 */ @@ -37,4 +35,9 @@ public class LimitQueryBuilderTest extends BaseQueryTestCase protected LimitQueryBuilder doCreateTestQueryBuilder() { return new LimitQueryBuilder(randomIntBetween(0, 20)); } + + @Override + protected void doAssertLuceneQuery(LimitQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + assertThat(query, instanceOf(MatchAllDocsQuery.class)); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/MatchAllQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/MatchAllQueryBuilderTest.java index d05ec6cbc94..f5e48ef9815 100644 --- a/core/src/test/java/org/elasticsearch/index/query/MatchAllQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/MatchAllQueryBuilderTest.java @@ -22,16 +22,19 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; -public class MatchAllQueryBuilderTest extends BaseQueryTestCase { +import java.io.IOException; - @Override - protected Query doCreateExpectedQuery(MatchAllQueryBuilder queryBuilder, QueryParseContext context) { - return new MatchAllDocsQuery(); - } +import static org.hamcrest.CoreMatchers.instanceOf; + +public class MatchAllQueryBuilderTest extends BaseQueryTestCase { @Override protected MatchAllQueryBuilder doCreateTestQueryBuilder() { return new MatchAllQueryBuilder(); } + @Override + protected void doAssertLuceneQuery(MatchAllQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + assertThat(query, instanceOf(MatchAllDocsQuery.class)); + } } \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/index/query/MissingQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/MissingQueryBuilderTest.java index 53c3ff44fa0..18b91a90367 100644 --- a/core/src/test/java/org/elasticsearch/index/query/MissingQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/MissingQueryBuilderTest.java @@ -19,15 +19,10 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.*; -import org.elasticsearch.common.lucene.search.Queries; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper; -import org.elasticsearch.index.mapper.object.ObjectMapper; +import org.apache.lucene.search.Query; import org.junit.Test; import java.io.IOException; -import java.util.Collection; import static org.hamcrest.Matchers.is; @@ -35,7 +30,7 @@ public class MissingQueryBuilderTest extends BaseQueryTestCase fields = context.simpleMatchToIndexNames(fieldPattern); - if (fields.isEmpty()) { - if (existence) { - // if we ask for existence of fields, and we found none, then we should match on all - return Queries.newMatchAllQuery(); - } - return null; - } - - Query existenceFilter = null; - Query nullFilter = null; - - if (existence) { - BooleanQuery boolFilter = new BooleanQuery(); - for (String field : fields) { - MappedFieldType fieldType = context.fieldMapper(field); - Query filter = null; - if (fieldNamesFieldType.isEnabled()) { - final String f; - if (fieldType != null) { - f = fieldType.names().indexName(); - } else { - f = field; - } - filter = fieldNamesFieldType.termQuery(f, context); - } - // if _field_names are not indexed, we need to go the slow way - if (filter == null && fieldType != null) { - filter = fieldType.rangeQuery(null, null, true, true); - } - if (filter == null) { - filter = new TermRangeQuery(field, null, null, true, true); - } - boolFilter.add(filter, BooleanClause.Occur.SHOULD); - } - - existenceFilter = boolFilter; - existenceFilter = Queries.not(existenceFilter);; - } - - if (nullValue) { - for (String field : fields) { - MappedFieldType fieldType = context.fieldMapper(field); - if (fieldType != null) { - nullFilter = fieldType.nullValueQuery(); - } - } - } - - Query filter; - if (nullFilter != null) { - if (existenceFilter != null) { - BooleanQuery combined = new BooleanQuery(); - combined.add(existenceFilter, BooleanClause.Occur.SHOULD); - combined.add(nullFilter, BooleanClause.Occur.SHOULD); - // cache the not filter as well, so it will be faster - filter = combined; - } else { - filter = nullFilter; - } - } else { - filter = existenceFilter; - } - - if (filter == null) { - return null; - } - - return new ConstantScoreQuery(filter); + protected void doAssertLuceneQuery(MissingQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + //too many mapping dependent cases to test, we don't want to end up duplication the toQuery method } @Test diff --git a/core/src/test/java/org/elasticsearch/index/query/NotQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/NotQueryBuilderTest.java index cadee9b889a..249df3d1add 100644 --- a/core/src/test/java/org/elasticsearch/index/query/NotQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/NotQueryBuilderTest.java @@ -19,23 +19,21 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; -import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.junit.Test; import java.io.IOException; -public class NotQueryBuilderTest extends BaseQueryTestCase { +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.nullValue; - @Override - protected Query doCreateExpectedQuery(NotQueryBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException { - if (queryBuilder.filter().toQuery(context) == null) { - return null; - } - return Queries.not(queryBuilder.filter().toQuery(context)); - } +public class NotQueryBuilderTest extends BaseQueryTestCase { /** * @return a NotQueryBuilder with random limit between 0 and 20 @@ -45,6 +43,22 @@ public class NotQueryBuilderTest extends BaseQueryTestCase { return new NotQueryBuilder(RandomQueryBuilder.createQuery(random())); } + @Override + protected void doAssertLuceneQuery(NotQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + Query filter = queryBuilder.filter().toQuery(context); + if (filter == null) { + assertThat(query, nullValue()); + } else { + assertThat(query, instanceOf(BooleanQuery.class)); + BooleanQuery booleanQuery = (BooleanQuery) query; + assertThat(booleanQuery.clauses().size(), equalTo(2)); + assertThat(booleanQuery.clauses().get(0).getOccur(), equalTo(BooleanClause.Occur.MUST)); + assertThat(booleanQuery.clauses().get(0).getQuery(), instanceOf(MatchAllDocsQuery.class)); + assertThat(booleanQuery.clauses().get(1).getOccur(), equalTo(BooleanClause.Occur.MUST_NOT)); + assertThat(booleanQuery.clauses().get(1).getQuery(), equalTo(filter)); + } + } + /** * @throws IOException */ diff --git a/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java index ed19a470ebd..b7495979971 100644 --- a/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java @@ -19,7 +19,7 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.BooleanClause.Occur; +import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.xcontent.XContentFactory; @@ -27,10 +27,18 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.junit.Test; import java.io.IOException; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.nullValue; @SuppressWarnings("deprecation") public class OrQueryBuilderTest extends BaseQueryTestCase { +/* @Override protected Query doCreateExpectedQuery(OrQueryBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException { if (queryBuilder.filters().isEmpty()) { @@ -49,6 +57,7 @@ public class OrQueryBuilderTest extends BaseQueryTestCase { } return query; } +*/ /** * @return an OrQueryBuilder with random limit between 0 and 20 @@ -63,6 +72,34 @@ public class OrQueryBuilderTest extends BaseQueryTestCase { return query; } + @Override + protected void doAssertLuceneQuery(OrQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + if (queryBuilder.filters().isEmpty()) { + assertThat(query, nullValue()); + } else { + List innerQueries = new ArrayList<>(); + for (QueryBuilder subQuery : queryBuilder.filters()) { + Query innerQuery = subQuery.toQuery(context); + // ignore queries that are null + if (innerQuery != null) { + innerQueries.add(innerQuery); + } + } + if (innerQueries.isEmpty()) { + assertThat(query, nullValue()); + } else { + assertThat(query, instanceOf(BooleanQuery.class)); + BooleanQuery booleanQuery = (BooleanQuery) query; + assertThat(booleanQuery.clauses().size(), equalTo(innerQueries.size())); + Iterator queryIterator = innerQueries.iterator(); + for (BooleanClause booleanClause : booleanQuery) { + assertThat(booleanClause.getOccur(), equalTo(BooleanClause.Occur.SHOULD)); + assertThat(booleanClause.getQuery(), equalTo(queryIterator.next())); + } + } + } + } + /** * test corner case where no inner queries exist */ diff --git a/core/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTest.java index c02394012b4..752fbcf6999 100644 --- a/core/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTest.java @@ -19,18 +19,14 @@ package org.elasticsearch.index.query; -import org.apache.lucene.index.Term; -import org.apache.lucene.search.MultiTermQuery; import org.apache.lucene.search.PrefixQuery; import org.apache.lucene.search.Query; -import org.elasticsearch.common.ParseFieldMatcher; -import org.elasticsearch.common.lucene.BytesRefs; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.query.support.QueryParsers; import org.junit.Test; import java.io.IOException; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; public class PrefixQueryBuilderTest extends BaseQueryTestCase { @@ -48,26 +44,10 @@ public class PrefixQueryBuilderTest extends BaseQueryTestCase { @Override - protected Query doCreateExpectedQuery(QueryFilterBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException { - Query query = queryBuilder.innerQuery().toQuery(context); - return query != null ? new ConstantScoreQuery(query) : query; - } - - /** - * @return a QueryFilterBuilder with random inner query - */ - @Override protected QueryFilterBuilder doCreateTestQueryBuilder() { QueryBuilder innerQuery = RandomQueryBuilder.createQuery(random()); return new QueryFilterBuilder(innerQuery); } + @Override + protected void doAssertLuceneQuery(QueryFilterBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + Query innerQuery = queryBuilder.innerQuery().toQuery(context); + if (innerQuery == null) { + assertThat(query, nullValue()); + } else { + assertThat(query, instanceOf(ConstantScoreQuery.class)); + ConstantScoreQuery constantScoreQuery = (ConstantScoreQuery) query; + assertThat(constantScoreQuery.getQuery(), equalTo(innerQuery)); + } + } + + @Override + protected boolean supportsBoostAndQueryName() { + return false; + } + + /** + * test wrapping an inner filter that returns null also returns null to pass on upwards + */ + @Test + public void testInnerQueryReturnsNull() throws IOException { + QueryParseContext context = createContext(); + + // create inner filter + String queryString = "{ \"constant_score\" : { \"filter\" : {} }"; + XContentParser parser = XContentFactory.xContent(queryString).createParser(queryString); + context.reset(parser); + assertQueryHeader(parser, ConstantScoreQueryBuilder.PROTOTYPE.getName()); + QueryBuilder innerQuery = context.indexQueryParserService().queryParser(ConstantScoreQueryBuilder.PROTOTYPE.getName()).fromXContent(context); + + // check that when wrapping this filter, toQuery() returns null + QueryFilterBuilder queryFilterQuery = new QueryFilterBuilder(innerQuery); + assertNull(queryFilterQuery.toQuery(createContext())); + } + @Test public void testValidate() { QueryBuilder innerQuery = null; diff --git a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTest.java index 4ee37613eec..2b90579dc93 100644 --- a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTest.java @@ -22,11 +22,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.NumericRangeQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermRangeQuery; -import org.elasticsearch.common.joda.DateMathParser; -import org.elasticsearch.common.joda.Joda; -import org.elasticsearch.common.lucene.BytesRefs; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.mapper.core.DateFieldMapper; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.junit.Test; @@ -35,6 +30,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; public class RangeQueryBuilderTest extends BaseQueryTestCase { @@ -93,32 +89,16 @@ public class RangeQueryBuilderTest extends BaseQueryTestCase } @Override - protected Query doCreateExpectedQuery(RangeQueryBuilder queryBuilder, QueryParseContext context) throws IOException { - Query expectedQuery; - String fieldName = queryBuilder.fieldName(); - if (getCurrentTypes().length == 0 || (fieldName.equals(DATE_FIELD_NAME) == false && fieldName.equals(INT_FIELD_NAME) == false)) { - expectedQuery = new TermRangeQuery(fieldName, BytesRefs.toBytesRef(queryBuilder.from()), - BytesRefs.toBytesRef(queryBuilder.to()), queryBuilder.includeLower(), queryBuilder.includeUpper()); - - } else if (fieldName.equals(DATE_FIELD_NAME)) { - DateMathParser forcedDateParser = null; - if (queryBuilder.format() != null) { - forcedDateParser = new DateMathParser(Joda.forPattern(queryBuilder.format())); - } - DateTimeZone dateTimeZone = null; - if (queryBuilder.timeZone() != null) { - dateTimeZone = DateTimeZone.forID(queryBuilder.timeZone()); - } - MappedFieldType mapper = context.fieldMapper(queryBuilder.fieldName()); - expectedQuery = ((DateFieldMapper.DateFieldType) mapper).rangeQuery(BytesRefs.toBytesRef(queryBuilder.from()), BytesRefs.toBytesRef(queryBuilder.to()), - queryBuilder.includeLower(), queryBuilder.includeUpper(), dateTimeZone, forcedDateParser); + protected void doAssertLuceneQuery(RangeQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + if (getCurrentTypes().length == 0 || (queryBuilder.fieldName().equals(DATE_FIELD_NAME) == false && queryBuilder.fieldName().equals(INT_FIELD_NAME) == false)) { + assertThat(query, instanceOf(TermRangeQuery.class)); + } else if (queryBuilder.fieldName().equals(DATE_FIELD_NAME)) { + //we can't properly test unmapped dates because LateParsingQuery is package private } else if (queryBuilder.fieldName().equals(INT_FIELD_NAME)) { - expectedQuery = NumericRangeQuery.newIntRange(INT_FIELD_NAME, (Integer) queryBuilder.from(), (Integer) queryBuilder.to(), - queryBuilder.includeLower(), queryBuilder.includeUpper()); + assertThat(query, instanceOf(NumericRangeQuery.class)); } else { throw new UnsupportedOperationException(); } - return expectedQuery; } @Test diff --git a/core/src/test/java/org/elasticsearch/index/query/RegexpQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/RegexpQueryBuilderTest.java index 30cfaccad79..153b2225625 100644 --- a/core/src/test/java/org/elasticsearch/index/query/RegexpQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/RegexpQueryBuilderTest.java @@ -19,20 +19,15 @@ package org.elasticsearch.index.query; -import org.apache.lucene.index.Term; -import org.apache.lucene.search.MultiTermQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.RegexpQuery; -import org.elasticsearch.common.ParseFieldMatcher; -import org.elasticsearch.common.lucene.BytesRefs; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.query.support.QueryParsers; import org.junit.Test; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; public class RegexpQueryBuilderTest extends BaseQueryTestCase { @@ -62,25 +57,8 @@ public class RegexpQueryBuilderTest extends BaseQueryTestCase { @@ -47,8 +48,8 @@ public class ScriptQueryBuilderTest extends BaseQueryTestCase fields = new TreeMap<>(); - // Use the default field (_all) if no fields specified - if (queryBuilder.fields().isEmpty()) { - String field = context.defaultField(); - fields.put(field, AbstractQueryBuilder.DEFAULT_BOOST); - } else { - fields.putAll(queryBuilder.fields()); - } - - // Use standard analyzer by default if none specified - Analyzer luceneAnalyzer; - if (queryBuilder.analyzer() == null) { - luceneAnalyzer = context.mapperService().searchAnalyzer(); - } else { - luceneAnalyzer = context.analysisService().analyzer(queryBuilder.analyzer()); - } - SimpleQueryParser sqp = new SimpleQueryParser(luceneAnalyzer, fields, queryBuilder.flags(), new Settings(queryBuilder.locale(), - queryBuilder.lowercaseExpandedTerms(), queryBuilder.lenient(), queryBuilder.analyzeWildcard())); - - if (queryBuilder.defaultOperator() != null) { - switch (queryBuilder.defaultOperator()) { - case OR: - sqp.setDefaultOperator(Occur.SHOULD); - break; - case AND: - sqp.setDefaultOperator(Occur.MUST); - break; - } - } - - Query query = sqp.parse(queryBuilder.text()); - if (queryBuilder.minimumShouldMatch() != null && query instanceof BooleanQuery) { - Queries.applyMinimumShouldMatch((BooleanQuery) query, queryBuilder.minimumShouldMatch()); - } - return query; - } - } diff --git a/core/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTest.java index 22a1ab0a678..fad6eb802dd 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTest.java @@ -21,19 +21,13 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.Query; import org.apache.lucene.search.spans.SpanContainingQuery; -import org.apache.lucene.search.spans.SpanQuery; import org.junit.Test; import java.io.IOException; -public class SpanContainingQueryBuilderTest extends BaseQueryTestCase { +import static org.hamcrest.CoreMatchers.instanceOf; - @Override - protected Query doCreateExpectedQuery(SpanContainingQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { - SpanQuery big = (SpanQuery) testQueryBuilder.big().toQuery(context); - SpanQuery little = (SpanQuery) testQueryBuilder.little().toQuery(context); - return new SpanContainingQuery(big, little); - } +public class SpanContainingQueryBuilderTest extends BaseQueryTestCase { @Override protected SpanContainingQueryBuilder doCreateTestQueryBuilder() { @@ -41,6 +35,11 @@ public class SpanContainingQueryBuilderTest extends BaseQueryTestCase { - @Override - protected Query doCreateExpectedQuery(SpanFirstQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { - SpanQuery innerQuery = (SpanQuery) testQueryBuilder.matchBuilder().toQuery(context); - return new SpanFirstQuery(innerQuery, testQueryBuilder.end()); - } - @Override protected SpanFirstQueryBuilder doCreateTestQueryBuilder() { SpanTermQueryBuilder[] spanTermQueries = new SpanTermQueryBuilderTest().createSpanTermQueryBuilders(1); return new SpanFirstQueryBuilder(spanTermQueries[0], randomIntBetween(0, 1000)); } + @Override + protected void doAssertLuceneQuery(SpanFirstQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + assertThat(query, instanceOf(SpanFirstQuery.class)); + } + @Test public void testValidate() { int totalExpectedErrors = 0; diff --git a/core/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTest.java index 87734fd8304..a3e2d264049 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTest.java @@ -26,13 +26,10 @@ import org.junit.Test; import java.io.IOException; -public class SpanMultiTermQueryBuilderTest extends BaseQueryTestCase { +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; - @Override - protected Query doCreateExpectedQuery(SpanMultiTermQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { - Query multiTermQuery = testQueryBuilder.multiTermQueryBuilder().toQuery(context); - return new SpanMultiTermQueryWrapper<>((MultiTermQuery) multiTermQuery); - } +public class SpanMultiTermQueryBuilderTest extends BaseQueryTestCase { @Override protected SpanMultiTermQueryBuilder doCreateTestQueryBuilder() { @@ -40,6 +37,15 @@ public class SpanMultiTermQueryBuilderTest extends BaseQueryTestCase((MultiTermQuery)multiTermQuery).getWrappedQuery())); + } + @Test public void testValidate() { int totalExpectedErrors = 0; diff --git a/core/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTest.java index 5a1742cb6f3..d1c3db7012b 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTest.java @@ -25,23 +25,13 @@ import org.apache.lucene.search.spans.SpanQuery; import org.junit.Test; import java.io.IOException; -import java.util.List; +import java.util.Iterator; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; public class SpanNearQueryBuilderTest extends BaseQueryTestCase { - @Override - protected Query doCreateExpectedQuery(SpanNearQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { - List clauses = testQueryBuilder.clauses(); - SpanQuery[] spanQueries = new SpanQuery[clauses.size()]; - for (int i = 0; i < clauses.size(); i++) { - Query query = clauses.get(i).toQuery(context); - assert query instanceof SpanQuery; - spanQueries[i] = (SpanQuery) query; - } - return new SpanNearQuery(spanQueries, testQueryBuilder.slop(), testQueryBuilder.inOrder(), testQueryBuilder.collectPayloads()); - - } - @Override protected SpanNearQueryBuilder doCreateTestQueryBuilder() { SpanNearQueryBuilder queryBuilder = new SpanNearQueryBuilder(randomIntBetween(-10, 10)); @@ -54,6 +44,19 @@ public class SpanNearQueryBuilderTest extends BaseQueryTestCase spanQueryBuilderIterator = queryBuilder.clauses().iterator(); + for (SpanQuery spanQuery : spanNearQuery.getClauses()) { + assertThat(spanQuery, equalTo(spanQueryBuilderIterator.next().toQuery(context))); + } + } + @Test public void testValidate() { SpanNearQueryBuilder queryBuilder = new SpanNearQueryBuilder(1); diff --git a/core/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTest.java index 03688371d9a..1b8b9993dcd 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTest.java @@ -21,7 +21,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.Query; import org.apache.lucene.search.spans.SpanNotQuery; -import org.apache.lucene.search.spans.SpanQuery; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; @@ -33,16 +32,10 @@ import static org.elasticsearch.index.query.QueryBuilders.spanNearQuery; import static org.elasticsearch.index.query.QueryBuilders.spanTermQuery; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; public class SpanNotQueryBuilderTest extends BaseQueryTestCase { - @Override - protected Query doCreateExpectedQuery(SpanNotQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { - SpanQuery include = (SpanQuery) testQueryBuilder.include().toQuery(context); - SpanQuery exclude = (SpanQuery) testQueryBuilder.exclude().toQuery(context); - return new SpanNotQuery(include, exclude, testQueryBuilder.pre(), testQueryBuilder.post()); - } - @Override protected SpanNotQueryBuilder doCreateTestQueryBuilder() { SpanTermQueryBuilder[] spanTermQueries = new SpanTermQueryBuilderTest().createSpanTermQueryBuilders(2); @@ -61,6 +54,14 @@ public class SpanNotQueryBuilderTest extends BaseQueryTestCase { - @Override - protected Query doCreateExpectedQuery(SpanOrQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { - List clauses = testQueryBuilder.clauses(); - SpanQuery[] spanQueries = new SpanQuery[clauses.size()]; - for (int i = 0; i < clauses.size(); i++) { - Query query = clauses.get(i).toQuery(context); - assert query instanceof SpanQuery; - spanQueries[i] = (SpanQuery) query; - } - return new SpanOrQuery(spanQueries); - } - @Override protected SpanOrQueryBuilder doCreateTestQueryBuilder() { SpanOrQueryBuilder queryBuilder = new SpanOrQueryBuilder(); @@ -51,6 +42,17 @@ public class SpanOrQueryBuilderTest extends BaseQueryTestCase spanQueryBuilderIterator = queryBuilder.clauses().iterator(); + for (SpanQuery spanQuery : spanOrQuery.getClauses()) { + assertThat(spanQuery, equalTo(spanQueryBuilderIterator.next().toQuery(context))); + } + } + @Test public void testValidate() { SpanOrQueryBuilder queryBuilder = new SpanOrQueryBuilder(); diff --git a/core/src/test/java/org/elasticsearch/index/query/SpanTermQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/SpanTermQueryBuilderTest.java index e487a3532ba..6de939cf98b 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SpanTermQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/SpanTermQueryBuilderTest.java @@ -19,9 +19,16 @@ package org.elasticsearch.index.query; -import org.apache.lucene.index.Term; import org.apache.lucene.search.Query; import org.apache.lucene.search.spans.SpanTermQuery; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.lucene.BytesRefs; +import org.elasticsearch.index.mapper.MappedFieldType; + +import java.io.IOException; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; public class SpanTermQueryBuilderTest extends BaseTermQueryTestCase { @@ -31,8 +38,17 @@ public class SpanTermQueryBuilderTest extends BaseTermQueryTestCase { +import static org.hamcrest.CoreMatchers.instanceOf; - @Override - protected Query doCreateExpectedQuery(SpanWithinQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { - SpanQuery big = (SpanQuery) testQueryBuilder.big().toQuery(context); - SpanQuery little = (SpanQuery) testQueryBuilder.little().toQuery(context); - return new SpanWithinQuery(big, little); - } +public class SpanWithinQueryBuilderTest extends BaseQueryTestCase { @Override protected SpanWithinQueryBuilder doCreateTestQueryBuilder() { @@ -41,6 +35,11 @@ public class SpanWithinQueryBuilderTest extends BaseQueryTestCase { @@ -34,7 +41,16 @@ public class TermQueryBuilderTest extends BaseTermQueryTestCase { @@ -38,16 +37,15 @@ public class TypeQueryBuilderTest extends BaseQueryTestCase { } @Override - protected Query doCreateExpectedQuery(TypeQueryBuilder queryBuilder, QueryParseContext context) throws IOException { - Query expectedQuery; - //LUCENE 4 UPGRADE document mapper should use bytesref as well? - DocumentMapper documentMapper = context.mapperService().documentMapper(queryBuilder.type().utf8ToString()); - if (documentMapper == null) { - expectedQuery = new TermQuery(new Term(TypeFieldMapper.NAME, queryBuilder.type())); - } else { - expectedQuery = documentMapper.typeFilter(); + protected void doAssertLuceneQuery(TypeQueryBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { + assertThat(query, either(instanceOf(TermQuery.class)).or(instanceOf(ConstantScoreQuery.class))); + if (query instanceof ConstantScoreQuery) { + query = ((ConstantScoreQuery) query).getQuery(); + assertThat(query, instanceOf(TermQuery.class)); } - return expectedQuery; + TermQuery termQuery = (TermQuery) query; + assertThat(termQuery.getTerm().field(), equalTo(TypeFieldMapper.NAME)); + assertThat(termQuery.getTerm().bytes(), equalTo(queryBuilder.type())); } @Test diff --git a/core/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTest.java index dc3c6789f5f..3e9f54dc972 100644 --- a/core/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTest.java @@ -19,18 +19,13 @@ package org.elasticsearch.index.query; -import org.apache.lucene.index.Term; -import org.apache.lucene.search.MultiTermQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.WildcardQuery; -import org.apache.lucene.util.BytesRef; -import org.elasticsearch.common.ParseFieldMatcher; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.query.support.QueryParsers; import org.junit.Test; import java.io.IOException; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; public class WildcardQueryBuilderTest extends BaseQueryTestCase { @@ -53,26 +48,8 @@ public class WildcardQueryBuilderTest extends BaseQueryTestCase