From 7281f01b3307cc20d27e742e161d4b834ed87319 Mon Sep 17 00:00:00 2001 From: Alex Ksikes Date: Sat, 4 Jul 2015 19:00:14 +0200 Subject: [PATCH] Refactors TypeQuery Relates to #10217 Closes #12035 This PR is against the query-refactoring branch. --- .../index/query/TypeQueryBuilder.java | 68 ++++++++++++++++++- .../index/query/TypeQueryParser.java | 32 +++------ .../index/query/BaseQueryTestCase.java | 37 ++++++---- .../index/query/TypeQueryBuilderTest.java | 58 ++++++++++++++++ 4 files changed, 154 insertions(+), 41 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/index/query/TypeQueryBuilderTest.java diff --git a/core/src/main/java/org/elasticsearch/index/query/TypeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TypeQueryBuilder.java index a86b9c7496d9e..0dcaffa385814 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TypeQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TypeQueryBuilder.java @@ -19,24 +19,44 @@ package org.elasticsearch.index.query; +import org.apache.lucene.index.Term; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermQuery; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.internal.TypeFieldMapper; import java.io.IOException; +import java.util.Objects; public class TypeQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "type"; - private final String type; - static final TypeQueryBuilder PROTOTYPE = new TypeQueryBuilder(null); + + private final BytesRef type; + + static final TypeQueryBuilder PROTOTYPE = new TypeQueryBuilder((BytesRef) null); public TypeQueryBuilder(String type) { + this.type = BytesRefs.toBytesRef(type); + } + + TypeQueryBuilder(BytesRef type) { this.type = type; } + + public BytesRef type() { + return this.type; + } @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); - builder.field("value", type); + builder.field("value", type.utf8ToString()); printBoostAndQueryName(builder); builder.endObject(); } @@ -45,4 +65,46 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep public String getName() { return NAME; } + + @Override + protected Query doToQuery(QueryParseContext parseContext) throws IOException { + Query filter; + //LUCENE 4 UPGRADE document mapper should use bytesref as well? + DocumentMapper documentMapper = parseContext.mapperService().documentMapper(type.utf8ToString()); + if (documentMapper == null) { + filter = new TermQuery(new Term(TypeFieldMapper.NAME, type)); + } else { + filter = documentMapper.typeFilter(); + } + return filter; + } + + @Override + public QueryValidationException validate() { + QueryValidationException validationException = null; + if (type == null) { + validationException = addValidationError("[type] cannot be null", validationException); + } + return validationException; + } + + @Override + protected TypeQueryBuilder doReadFrom(StreamInput in) throws IOException { + return new TypeQueryBuilder(in.readBytesRef()); + } + + @Override + protected void doWriteTo(StreamOutput out) throws IOException { + out.writeBytesRef(type); + } + + @Override + protected int doHashCode() { + return Objects.hash(type); + } + + @Override + protected boolean doEquals(TypeQueryBuilder other) { + return Objects.equals(type, other.type); + } } \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/index/query/TypeQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/TypeQueryParser.java index b81da6266e339..6fdb5ec32d7da 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TypeQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/TypeQueryParser.java @@ -19,18 +19,13 @@ package org.elasticsearch.index.query; -import org.apache.lucene.index.Term; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.mapper.DocumentMapper; -import org.elasticsearch.index.mapper.internal.TypeFieldMapper; import java.io.IOException; -public class TypeQueryParser extends BaseQueryParserTemp { +public class TypeQueryParser extends BaseQueryParser { @Inject public TypeQueryParser() { @@ -42,11 +37,13 @@ public String[] names() { } @Override - public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException { + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { XContentParser parser = parseContext.parser(); + BytesRef type = null; + String queryName = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; - BytesRef type = null; + String currentFieldName = null; XContentParser.Token token; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -68,22 +65,9 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars if (type == null) { throw new QueryParsingException(parseContext, "[type] filter needs to be provided with a value for the type"); } - - Query filter; - //LUCENE 4 UPGRADE document mapper should use bytesref as well? - DocumentMapper documentMapper = parseContext.mapperService().documentMapper(type.utf8ToString()); - if (documentMapper == null) { - filter = new TermQuery(new Term(TypeFieldMapper.NAME, type)); - } else { - filter = documentMapper.typeFilter(); - } - if (queryName != null) { - parseContext.addNamedQuery(queryName, filter); - } - if (filter != null) { - filter.setBoost(boost); - } - return filter; + return new TypeQueryBuilder(type) + .boost(boost) + .queryName(queryName); } @Override 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 af801bea25d05..8d879ff0cc793 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java @@ -160,20 +160,7 @@ public static void afterClass() throws Exception { @Before public void beforeTest() { //set some random types to be queried as part the search request, before each test - String[] types; - if (currentTypes.length > 0 && randomBoolean()) { - int numberOfQueryTypes = randomIntBetween(1, currentTypes.length); - types = new String[numberOfQueryTypes]; - for (int i = 0; i < numberOfQueryTypes; i++) { - types[i] = randomFrom(currentTypes); - } - } else { - if (randomBoolean()) { - types = new String[] { MetaData.ALL }; - } else { - types = new String[0]; - } - } + String[] types = getRandomTypes(); //some query (e.g. range query) have a different behaviour depending on whether the current search context is set or not //which is why we randomly set the search context, which will internally also do QueryParseContext.setTypes(types) if (randomBoolean()) { @@ -348,4 +335,26 @@ protected static String getRandomRewriteMethod() { } return rewrite; } + + protected String[] getRandomTypes() { + String[] types; + if (currentTypes.length > 0 && randomBoolean()) { + int numberOfQueryTypes = randomIntBetween(1, currentTypes.length); + types = new String[numberOfQueryTypes]; + for (int i = 0; i < numberOfQueryTypes; i++) { + types[i] = randomFrom(currentTypes); + } + } else { + if (randomBoolean()) { + types = new String[] { MetaData.ALL }; + } else { + types = new String[0]; + } + } + return types; + } + + protected String getRandomType() { + return (currentTypes.length == 0) ? MetaData.ALL : randomFrom(currentTypes); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/TypeQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/TypeQueryBuilderTest.java new file mode 100644 index 0000000000000..5d95e12c1108a --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/TypeQueryBuilderTest.java @@ -0,0 +1,58 @@ +/* + * 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.index.Term; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermQuery; +import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.internal.TypeFieldMapper; +import org.junit.Test; + +import java.io.IOException; + +import static org.hamcrest.Matchers.is; + +public class TypeQueryBuilderTest extends BaseQueryTestCase { + + @Override + protected TypeQueryBuilder doCreateTestQueryBuilder() { + return new TypeQueryBuilder(getRandomType()); + } + + @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(); + } + return expectedQuery; + } + + @Test + public void testValidate() { + TypeQueryBuilder typeQueryBuilder = new TypeQueryBuilder((String) null); + assertThat(typeQueryBuilder.validate().validationErrors().size(), is(1)); + } +}