diff --git a/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java b/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java index 9074a6d8a05..1052eb4e37c 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java @@ -19,9 +19,14 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.Query; +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; /** * A filter that simply wraps a query. Same as the {@link QueryFilterBuilder} except that it allows also to @@ -30,24 +35,105 @@ import java.io.IOException; * query as a filter directly. */ @Deprecated -public class FQueryFilterBuilder extends QueryFilterBuilder { +public class FQueryFilterBuilder extends AbstractQueryBuilder { public static final String NAME = "fquery"; static final FQueryFilterBuilder PROTOTYPE = new FQueryFilterBuilder(null); + private String queryName; + + private final QueryBuilder queryBuilder; + /** * A filter that simply wraps a query. * * @param queryBuilder The query to wrap as a filter */ public FQueryFilterBuilder(QueryBuilder queryBuilder) { - super(queryBuilder); + this.queryBuilder = queryBuilder; + } + + /** + * @return the query builder that is wrapped by this {@link FQueryFilterBuilder} + */ + public QueryBuilder innerQuery() { + return this.queryBuilder; + } + + /** + * Sets the query name for the filter that can be used when searching for matched_filters per hit. + */ + public FQueryFilterBuilder queryName(String queryName) { + this.queryName = queryName; + return this; + } + + /** + * @return the query name for the filter that can be used when searching for matched_filters per hit + */ + public String queryName() { + return this.queryName; } @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { - buildFQuery(builder, params); + builder.startObject(FQueryFilterBuilder.NAME); + builder.field("query"); + queryBuilder.toXContent(builder, params); + if (queryName != null) { + builder.field("_name", queryName); + } + builder.endObject(); + } + + @Override + public Query toQuery(QueryParseContext parseContext) throws QueryParsingException, IOException { + // inner query builder can potentially be `null`, in that case we ignore it + if (this.queryBuilder == null) { + return null; + } + Query innerQuery = this.queryBuilder.toQuery(parseContext); + if (innerQuery == null) { + return null; + } + Query query = new ConstantScoreQuery(innerQuery); + if (queryName != null) { + parseContext.addNamedQuery(queryName, query); + } + return query; + } + + @Override + public int hashCode() { + return Objects.hash(queryBuilder, queryName); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + FQueryFilterBuilder other = (FQueryFilterBuilder) obj; + return Objects.equals(queryBuilder, other.queryBuilder) && + Objects.equals(queryName, other.queryName); + } + + @Override + public FQueryFilterBuilder readFrom(StreamInput in) throws IOException { + QueryBuilder innerQueryBuilder = in.readNamedWriteable(); + FQueryFilterBuilder fquery = new FQueryFilterBuilder(innerQueryBuilder); + fquery.queryName = in.readOptionalString(); + return fquery; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeNamedWriteable(this.queryBuilder); + out.writeOptionalString(queryName); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/FQueryFilterParser.java b/core/src/main/java/org/elasticsearch/index/query/FQueryFilterParser.java index 672d232d179..f79e388a419 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FQueryFilterParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/FQueryFilterParser.java @@ -19,8 +19,6 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.Query; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.xcontent.XContentParser; @@ -31,7 +29,7 @@ import java.io.IOException; * associate a name with the query filter. */ @Deprecated -public class FQueryFilterParser extends BaseQueryParserTemp { +public class FQueryFilterParser extends BaseQueryParser { @Inject public FQueryFilterParser() { @@ -43,10 +41,10 @@ public class FQueryFilterParser extends BaseQueryParserTemp { } @Override - public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException { + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { XContentParser parser = parseContext.parser(); - Query query = null; + QueryBuilder wrappedQuery = null; boolean queryFound = false; String queryName = null; @@ -60,7 +58,7 @@ public class FQueryFilterParser extends BaseQueryParserTemp { } else if (token == XContentParser.Token.START_OBJECT) { if ("query".equals(currentFieldName)) { queryFound = true; - query = parseContext.parseInnerQuery(); + wrappedQuery = parseContext.parseInnerQueryBuilder(); } else { throw new QueryParsingException(parseContext, "[fquery] query does not support [" + currentFieldName + "]"); } @@ -75,14 +73,12 @@ public class FQueryFilterParser extends BaseQueryParserTemp { if (!queryFound) { throw new QueryParsingException(parseContext, "[fquery] requires 'query' element"); } - if (query == null) { + if (wrappedQuery == null) { return null; } - query = new ConstantScoreQuery(query); - if (queryName != null) { - parseContext.addNamedQuery(queryName, query); - } - return query; + FQueryFilterBuilder queryBuilder = new FQueryFilterBuilder(wrappedQuery); + queryBuilder.queryName(queryName); + return queryBuilder; } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java index 761a9de4178..a7122564048 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java @@ -19,9 +19,14 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.Query; +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; /** * A filter that simply wraps a query. @@ -35,8 +40,6 @@ public class QueryFilterBuilder extends AbstractQueryBuilder private final QueryBuilder queryBuilder; - private String queryName; - static final QueryFilterBuilder PROTOTYPE = new QueryFilterBuilder(null); /** @@ -49,32 +52,57 @@ public class QueryFilterBuilder extends AbstractQueryBuilder } /** - * Sets the query name for the filter that can be used when searching for matched_filters per hit. + * @return the query builder that is wrapped by this {@link QueryFilterBuilder} */ - public QueryFilterBuilder queryName(String queryName) { - this.queryName = queryName; - return this; + public QueryBuilder innerQuery() { + return this.queryBuilder; } @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { - if (queryName == null) { - builder.field(NAME); - queryBuilder.toXContent(builder, params); - } else { - //fallback fo fquery when needed, for bw comp - buildFQuery(builder, params); - } + builder.field(NAME); + queryBuilder.toXContent(builder, params); } - protected void buildFQuery(XContentBuilder builder, Params params) throws IOException { - builder.startObject(FQueryFilterBuilder.NAME); - builder.field("query"); - queryBuilder.toXContent(builder, params); - if (queryName != null) { - builder.field("_name", queryName); + @Override + public Query toQuery(QueryParseContext parseContext) throws QueryParsingException, IOException { + // inner query builder can potentially be `null`, in that case we ignore it + if (this.queryBuilder == null) { + return null; } - builder.endObject(); + Query innerQuery = this.queryBuilder.toQuery(parseContext); + if (innerQuery == null) { + return null; + } + return new ConstantScoreQuery(innerQuery); + } + + @Override + public int hashCode() { + return Objects.hash(queryBuilder); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + QueryFilterBuilder other = (QueryFilterBuilder) obj; + return Objects.equals(queryBuilder, other.queryBuilder); + } + + @Override + public QueryFilterBuilder readFrom(StreamInput in) throws IOException { + QueryBuilder innerQueryBuilder = in.readNamedWriteable(); + return new QueryFilterBuilder(innerQueryBuilder); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeNamedWriteable(this.queryBuilder); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryFilterParser.java b/core/src/main/java/org/elasticsearch/index/query/QueryFilterParser.java index 387ae01254c..3a5d0ab1fba 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryFilterParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryFilterParser.java @@ -19,14 +19,12 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.Query; import org.elasticsearch.common.inject.Inject; import java.io.IOException; @Deprecated -public class QueryFilterParser extends BaseQueryParserTemp { +public class QueryFilterParser extends BaseQueryParser { @Inject public QueryFilterParser() { @@ -38,8 +36,8 @@ public class QueryFilterParser extends BaseQueryParserTemp { } @Override - public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException { - return new ConstantScoreQuery(parseContext.parseInnerQuery()); + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { + return new QueryFilterBuilder(parseContext.parseInnerQueryBuilder()); } @Override diff --git a/core/src/test/java/org/elasticsearch/index/query/FQueryFilterBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/FQueryFilterBuilderTest.java new file mode 100644 index 00000000000..057c413f6b7 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/FQueryFilterBuilderTest.java @@ -0,0 +1,84 @@ +/* + * 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.ConstantScoreQuery; +import org.apache.lucene.search.Query; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.junit.Test; + +import java.io.IOException; + +import static org.hamcrest.Matchers.equalTo; + +@SuppressWarnings("deprecation") +public class FQueryFilterBuilderTest extends BaseQueryTestCase { + + @Override + protected Query createExpectedQuery(FQueryFilterBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException { + return new ConstantScoreQuery(queryBuilder.innerQuery().toQuery(context)); + } + + /** + * @return a AndQueryBuilder with random limit between 0 and 20 + */ + @Override + protected FQueryFilterBuilder createTestQueryBuilder() { + QueryBuilder innerQuery = RandomQueryBuilder.create(random()); + FQueryFilterBuilder testQuery = new FQueryFilterBuilder(innerQuery); + testQuery.queryName(randomAsciiOfLengthBetween(1, 10)); + return testQuery; + } + + @Override + protected void assertLuceneQuery(FQueryFilterBuilder queryBuilder, Query query, QueryParseContext context) { + Query namedQuery = context.copyNamedFilters().get(queryBuilder.queryName()); + assertThat(namedQuery, equalTo(query)); + } + + /** + * test corner case where no inner query exist + */ + @Test + public void testNoInnerQuery() throws QueryParsingException, IOException { + FQueryFilterBuilder queryFilterQuery = new FQueryFilterBuilder(null); + assertNull(queryFilterQuery.toQuery(createContext())); + } + + /** + * 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 + FQueryFilterBuilder queryFilterQuery = new FQueryFilterBuilder(innerQuery); + assertNull(queryFilterQuery.toQuery(createContext())); + } +} diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java new file mode 100644 index 00000000000..93e1ca0b6df --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.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.index.query; + +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.Query; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.junit.Test; + +import java.io.IOException; + +@SuppressWarnings("deprecation") +public class QueryFilterBuilderTest extends BaseQueryTestCase { + + @Override + protected Query createExpectedQuery(QueryFilterBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException { + return new ConstantScoreQuery(queryBuilder.innerQuery().toQuery(context)); + } + + /** + * @return a AndQueryBuilder with random limit between 0 and 20 + */ + @Override + protected QueryFilterBuilder createTestQueryBuilder() { + QueryBuilder innerQuery = RandomQueryBuilder.create(random()); + QueryFilterBuilder testQuery = new QueryFilterBuilder(innerQuery); + return testQuery; + } + + /** + * test corner case where no inner query exist + */ + @Test + public void testNoInnerQuery() throws QueryParsingException, IOException { + QueryFilterBuilder queryFilterQuery = new QueryFilterBuilder(null); + assertNull(queryFilterQuery.toQuery(createContext())); + } + + /** + * 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())); + } +} diff --git a/docs/reference/migration/migrate_query_refactoring.asciidoc b/docs/reference/migration/migrate_query_refactoring.asciidoc new file mode 100644 index 00000000000..99d2a01b4a6 --- /dev/null +++ b/docs/reference/migration/migrate_query_refactoring.asciidoc @@ -0,0 +1,13 @@ +[[breaking-changes query-refactoring]] +== Breaking changes on the query-refactoring branch + +This section discusses changes that are breaking to the current rest or java-api +on the query-refactoring feature branch. + +=== Java-API + +==== QueryFilterBuilder + +Removed the setter `queryName(String queryName)` since this field is not supported +in this type of query. Use `FQueryFilterBuilder.queryName(String queryName)` instead +when in need to wrap a named query as a filter.