diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java b/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java index 55789b8df4d..5292b529d88 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java @@ -329,9 +329,13 @@ public abstract class QueryBuilders { return new SpanWithinQueryBuilder(); } - /** Creates a new {@code span_containing} builder. */ - public static SpanContainingQueryBuilder spanContainingQuery() { - return new SpanContainingQueryBuilder(); + /** + * Creates a new {@code span_containing} builder. + * @param big the big clause, it must enclose {@code little} for a match. + * @param little the little clause, it must be contained within {@code big} for a match. + */ + public static SpanContainingQueryBuilder spanContainingQuery(SpanQueryBuilder big, SpanQueryBuilder little) { + return new SpanContainingQueryBuilder(big, little); } /** diff --git a/core/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java index dd68266860d..91eb7194112 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java @@ -19,9 +19,15 @@ 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.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Objects; /** * Builder for {@link org.apache.lucene.search.spans.SpanContainingQuery}. @@ -29,46 +35,92 @@ import java.io.IOException; public class SpanContainingQueryBuilder extends AbstractQueryBuilder implements SpanQueryBuilder { public static final String NAME = "span_containing"; - private SpanQueryBuilder big; - private SpanQueryBuilder little; + private final SpanQueryBuilder big; + private final SpanQueryBuilder little; static final SpanContainingQueryBuilder PROTOTYPE = new SpanContainingQueryBuilder(); /** - * Sets the little clause, it must be contained within {@code big} for a match. + * @param big the big clause, it must enclose {@code little} for a match. + * @param little the little clause, it must be contained within {@code big} for a match. */ - public SpanContainingQueryBuilder little(SpanQueryBuilder clause) { - this.little = clause; - return this; + public SpanContainingQueryBuilder(SpanQueryBuilder big, SpanQueryBuilder little) { + this.little = Objects.requireNonNull(little); + this.big = Objects.requireNonNull(big); } /** - * Sets the big clause, it must enclose {@code little} for a match. + * only used for prototype */ - public SpanContainingQueryBuilder big(SpanQueryBuilder clause) { - this.big = clause; - return this; + private SpanContainingQueryBuilder() { + this.little = null; + this.big = null; + } + + /** + * @return the big clause, it must enclose {@code little} for a match. + */ + public SpanQueryBuilder big() { + return this.big; + } + + /** + * @return the little clause, it must be contained within {@code big} for a match. + */ + public SpanQueryBuilder little() { + return this.little; } @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { - if (big == null) { - throw new IllegalArgumentException("Must specify big clause when building a span_containing query"); - } - if (little == null) { - throw new IllegalArgumentException("Must specify little clause when building a span_containing query"); - } builder.startObject(NAME); - builder.field("big"); big.toXContent(builder, params); - builder.field("little"); little.toXContent(builder, params); - printBoostAndQueryName(builder); builder.endObject(); } + @Override + protected Query doToQuery(QueryParseContext parseContext) throws IOException { + Query innerBig = big.toQuery(parseContext); + assert innerBig instanceof SpanQuery; + Query innerLittle = little.toQuery(parseContext); + assert innerLittle instanceof SpanQuery; + return new SpanContainingQuery((SpanQuery) innerBig, (SpanQuery) innerLittle); + } + + @Override + public QueryValidationException validate() { + QueryValidationException validationExceptions = validateInnerQuery(big, null); + validationExceptions = validateInnerQuery(little, validationExceptions); + return validationExceptions; + } + + @Override + protected SpanContainingQueryBuilder doReadFrom(StreamInput in) throws IOException { + SpanQueryBuilder big = in.readNamedWriteable(); + SpanQueryBuilder little = in.readNamedWriteable(); + return new SpanContainingQueryBuilder(big, little); + } + + @Override + protected void doWriteTo(StreamOutput out) throws IOException { + out.writeNamedWriteable(big); + out.writeNamedWriteable(little); + } + + @Override + protected int doHashCode() { + return Objects.hash(big, little); + } + + @Override + protected boolean doEquals(SpanContainingQueryBuilder other) { + return Objects.equals(big, other.big) && + Objects.equals(little, other.little); + } + @Override public String getName() { return NAME; diff --git a/core/src/main/java/org/elasticsearch/index/query/SpanContainingQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/SpanContainingQueryParser.java index bc3f1a34207..c4052511c8f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SpanContainingQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/SpanContainingQueryParser.java @@ -19,9 +19,7 @@ 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.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.xcontent.XContentParser; @@ -31,7 +29,7 @@ import java.io.IOException; /** * Parser for {@link SpanContainingQuery} */ -public class SpanContainingQueryParser extends BaseQueryParserTemp { +public class SpanContainingQueryParser extends BaseQueryParser { @Inject public SpanContainingQueryParser() { @@ -43,13 +41,12 @@ public class SpanContainingQueryParser extends BaseQueryParserTemp { } @Override - public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException { + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { XContentParser parser = parseContext.parser(); - float boost = AbstractQueryBuilder.DEFAULT_BOOST; String queryName = null; - SpanQuery big = null; - SpanQuery little = null; + SpanQueryBuilder big = null; + SpanQueryBuilder little = null; String currentFieldName = null; XContentParser.Token token; @@ -58,17 +55,17 @@ public class SpanContainingQueryParser extends BaseQueryParserTemp { currentFieldName = parser.currentName(); } else if (token == XContentParser.Token.START_OBJECT) { if ("big".equals(currentFieldName)) { - Query query = parseContext.parseInnerQuery(); - if (!(query instanceof SpanQuery)) { + QueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (!(query instanceof SpanQueryBuilder)) { throw new QueryParsingException(parseContext, "span_containing [big] must be of type span query"); } - big = (SpanQuery) query; + big = (SpanQueryBuilder) query; } else if ("little".equals(currentFieldName)) { - Query query = parseContext.parseInnerQuery(); - if (!(query instanceof SpanQuery)) { + QueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (!(query instanceof SpanQueryBuilder)) { throw new QueryParsingException(parseContext, "span_containing [little] must be of type span query"); } - little = (SpanQuery) query; + little = (SpanQueryBuilder) query; } else { throw new QueryParsingException(parseContext, "[span_containing] query does not support [" + currentFieldName + "]"); } @@ -81,18 +78,8 @@ public class SpanContainingQueryParser extends BaseQueryParserTemp { } } - if (big == null) { - throw new QueryParsingException(parseContext, "span_containing must include [big]"); - } - if (little == null) { - throw new QueryParsingException(parseContext, "span_containing must include [little]"); - } - - Query query = new SpanContainingQuery(big, little); - query.setBoost(boost); - if (queryName != null) { - parseContext.addNamedQuery(queryName, query); - } + SpanContainingQueryBuilder query = new SpanContainingQueryBuilder(big, little); + query.boost(boost).queryName(queryName); return query; } diff --git a/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java b/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java index 59c2ce3cb19..d0bdccebc25 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java @@ -1344,10 +1344,7 @@ public class SimpleIndexQueryParserTests extends ElasticsearchSingleNodeTest { IndexQueryParserService queryParser = queryParser(); Query expectedQuery = new SpanContainingQuery(new SpanTermQuery(new Term("age", longToPrefixCoded(34, 0))), new SpanTermQuery(new Term("age", longToPrefixCoded(35, 0)))); - Query actualQuery = queryParser.parse(spanContainingQuery() - .big(spanTermQuery("age", 34)) - .little(spanTermQuery("age", 35))) - .query(); + Query actualQuery = queryParser.parse(spanContainingQuery(spanTermQuery("age", 34), spanTermQuery("age", 35))).query(); assertEquals(expectedQuery, actualQuery); } diff --git a/core/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTest.java new file mode 100644 index 00000000000..c0c9a81118a --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTest.java @@ -0,0 +1,87 @@ +/* + * 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.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 { + + @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); + } + + @Override + protected SpanContainingQueryBuilder doCreateTestQueryBuilder() { + SpanTermQueryBuilder bigQuery = new SpanTermQueryBuilderTest().createTestQueryBuilder(); + // we need same field name and value type as bigQuery for little query + Object bigValue = bigQuery.value; + Object littleValue; + if (bigValue instanceof Boolean) { + littleValue = randomBoolean(); + } else if (bigValue instanceof Integer) { + littleValue = randomInt(); + } else if (bigValue instanceof Double) { + littleValue = randomDouble(); + } else { + littleValue = randomAsciiOfLengthBetween(1, 10); + } + SpanTermQueryBuilder littleQuery = new SpanTermQueryBuilder(bigQuery.fieldName(), littleValue); + return new SpanContainingQueryBuilder(bigQuery, littleQuery); + } + + @Test + public void testValidate() { + int totalExpectedErrors = 0; + SpanQueryBuilder bigSpanQueryBuilder; + if (randomBoolean()) { + bigSpanQueryBuilder = new SpanTermQueryBuilder("", "test"); + totalExpectedErrors++; + } else { + bigSpanQueryBuilder = new SpanTermQueryBuilder("name", "value"); + } + SpanQueryBuilder littleSpanQueryBuilder; + if (randomBoolean()) { + littleSpanQueryBuilder = new SpanTermQueryBuilder("", "test"); + totalExpectedErrors++; + } else { + littleSpanQueryBuilder = new SpanTermQueryBuilder("name", "value"); + } + SpanContainingQueryBuilder queryBuilder = new SpanContainingQueryBuilder(bigSpanQueryBuilder, littleSpanQueryBuilder); + assertValidate(queryBuilder, totalExpectedErrors); + } + + @Test(expected=NullPointerException.class) + public void testNullBig() { + new SpanContainingQueryBuilder(null, new SpanTermQueryBuilder("name", "value")); + } + + @Test(expected=NullPointerException.class) + public void testNullLittle() { + new SpanContainingQueryBuilder(new SpanTermQueryBuilder("name", "value"), null); + } +} diff --git a/docs/reference/migration/migrate_query_refactoring.asciidoc b/docs/reference/migration/migrate_query_refactoring.asciidoc index 464e43e1982..7962d70d6b1 100644 --- a/docs/reference/migration/migrate_query_refactoring.asciidoc +++ b/docs/reference/migration/migrate_query_refactoring.asciidoc @@ -11,6 +11,12 @@ on the query-refactoring feature branch. Removed setters for mandatory positive/negative query. Both arguments now have to be supplied at construction time already and have to be non-null. +==== SpanContainingQueryBuilder + +Removed setters for mandatory big/little inner span queries. Both arguments now have +to be supplied at construction time already and have to be non-null. Updated +static factory methods in QueryBuilders accordingly. + ==== QueryFilterBuilder Removed the setter `queryName(String queryName)` since this field is not supported