From f7b3dce4bcca78745f6af73e03ceb5009c9448b5 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 3 Aug 2016 19:19:45 +0200 Subject: [PATCH] Throw parsing error if match_phrase_prefix query contains multiple fields Match phrase prefix Query, like many other queries, used to parse even when the query referred to multiple fields and the first one would win. We rather throw an exception now instead. Also added test for short prefix query variant and modified the parsing code to consume the whole query object. --- .../query/MatchPhrasePrefixQueryBuilder.java | 83 +++++++++---------- .../MatchPhrasePrefixQueryBuilderTests.java | 45 +++++++++- 2 files changed, 80 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilder.java index b53f10989dc..ecce9e66b10 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilder.java @@ -192,62 +192,55 @@ public class MatchPhrasePrefixQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); - - XContentParser.Token token = parser.nextToken(); - if (token != XContentParser.Token.FIELD_NAME) { - throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] query malformed, no field"); - } - String fieldName = parser.currentName(); - + String fieldName = null; Object value = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; String analyzer = null; int slop = MatchQuery.DEFAULT_PHRASE_SLOP; int maxExpansion = FuzzyQuery.defaultMaxExpansions; String queryName = null; - - token = parser.nextToken(); - if (token == XContentParser.Token.START_OBJECT) { - String currentFieldName = null; - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { - if (token == XContentParser.Token.FIELD_NAME) { - currentFieldName = parser.currentName(); - } else if (token.isValue()) { - if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchQueryBuilder.QUERY_FIELD)) { - value = parser.objectText(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchQueryBuilder.ANALYZER_FIELD)) { - analyzer = parser.text(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) { - boost = parser.floatValue(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchPhraseQueryBuilder.SLOP_FIELD)) { - slop = parser.intValue(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MAX_EXPANSIONS_FIELD)) { - maxExpansion = parser.intValue(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) { - queryName = parser.text(); + XContentParser.Token token; + String currentFieldName = null; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else if (parseContext.isDeprecatedSetting(currentFieldName)) { + // skip + } else if (token == XContentParser.Token.START_OBJECT) { + if (fieldName != null) { + throw new ParsingException(parser.getTokenLocation(), "[match_phrase_prefix] query doesn't support multiple " + + "fields, found [" + fieldName + "] and [" + currentFieldName + "]"); + } + fieldName = currentFieldName; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else if (token.isValue()) { + if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchQueryBuilder.QUERY_FIELD)) { + value = parser.objectText(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchQueryBuilder.ANALYZER_FIELD)) { + analyzer = parser.text(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) { + boost = parser.floatValue(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchPhraseQueryBuilder.SLOP_FIELD)) { + slop = parser.intValue(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MAX_EXPANSIONS_FIELD)) { + maxExpansion = parser.intValue(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) { + queryName = parser.text(); + } else { + throw new ParsingException(parser.getTokenLocation(), + "[" + NAME + "] query does not support [" + currentFieldName + "]"); + } } else { throw new ParsingException(parser.getTokenLocation(), - "[" + NAME + "] query does not support [" + currentFieldName + "]"); + "[" + NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } - } else { - throw new ParsingException(parser.getTokenLocation(), - "[" + NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } + } else { + fieldName = parser.currentName(); + value = parser.objectText(); } - parser.nextToken(); - } else { - value = parser.objectText(); - // move to the next token - token = parser.nextToken(); - if (token != XContentParser.Token.END_OBJECT) { - throw new ParsingException(parser.getTokenLocation(), "[" + NAME - + "] query parsed in simplified form, with direct field name, " - + "but included more options than just the field name, possibly use its 'options' form, with 'query' element?"); - } - } - - if (value == null) { - throw new ParsingException(parser.getTokenLocation(), "No text specified for text query"); } MatchPhrasePrefixQueryBuilder matchQuery = new MatchPhrasePrefixQueryBuilder(fieldName, value); diff --git a/core/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java index f5b2690db64..83a64ba8002 100644 --- a/core/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java @@ -23,11 +23,15 @@ import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.PointRangeQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.lucene.search.MatchNoDocsQuery; import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + import static org.hamcrest.CoreMatchers.either; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.Matchers.containsString; @@ -69,6 +73,20 @@ public class MatchPhrasePrefixQueryBuilderTests extends AbstractQueryTestCase getAlternateVersions() { + Map alternateVersions = new HashMap<>(); + MatchPhrasePrefixQueryBuilder matchPhrasePrefixQuery = new MatchPhrasePrefixQueryBuilder(randomAsciiOfLengthBetween(1, 10), + randomAsciiOfLengthBetween(1, 10)); + String contentString = "{\n" + + " \"match_phrase_prefix\" : {\n" + + " \"" + matchPhrasePrefixQuery.fieldName() + "\" : \"" + matchPhrasePrefixQuery.value() + "\"\n" + + " }\n" + + "}"; + alternateVersions.put(contentString, matchPhrasePrefixQuery); + return alternateVersions; + } + @Override protected void doAssertLuceneQuery(MatchPhrasePrefixQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { @@ -81,16 +99,16 @@ public class MatchPhrasePrefixQueryBuilderTests extends AbstractQueryTestCase