From 43fee1d7fa3029afb748e93ce3a07173c6cff76f Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 3 Aug 2016 20:01:21 +0200 Subject: [PATCH] Throw parsing error if fuzzy query contains multiple fields Fuzzy 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. --- .../index/query/FuzzyQueryBuilder.java | 88 +++++++++---------- .../index/query/FuzzyQueryBuilderTests.java | 43 ++++++++- 2 files changed, 82 insertions(+), 49 deletions(-) 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 5cf8275a076..aa0e6e2ff52 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java @@ -152,7 +152,7 @@ public class FuzzyQueryBuilder extends AbstractQueryBuilder i */ public FuzzyQueryBuilder(String fieldName, Object value) { if (Strings.isEmpty(fieldName)) { - throw new IllegalArgumentException("field name cannot be null or empty."); + throw new IllegalArgumentException("field name cannot be null or empty"); } if (value == null) { throw new IllegalArgumentException("query value cannot be null"); @@ -258,63 +258,59 @@ public class FuzzyQueryBuilder extends AbstractQueryBuilder i public static Optional 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(), "[fuzzy] query malformed, no field"); - } - - String fieldName = parser.currentName(); + String fieldName = null; Object value = null; - Fuzziness fuzziness = FuzzyQueryBuilder.DEFAULT_FUZZINESS; int prefixLength = FuzzyQueryBuilder.DEFAULT_PREFIX_LENGTH; int maxExpansions = FuzzyQueryBuilder.DEFAULT_MAX_EXPANSIONS; boolean transpositions = FuzzyQueryBuilder.DEFAULT_TRANSPOSITIONS; String rewrite = null; - String queryName = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; - - 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 (parseContext.getParseFieldMatcher().match(currentFieldName, TERM_FIELD)) { - value = parser.objectBytes(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, VALUE_FIELD)) { - value = parser.objectBytes(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) { - boost = parser.floatValue(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, Fuzziness.FIELD)) { - fuzziness = Fuzziness.parse(parser); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, PREFIX_LENGTH_FIELD)) { - prefixLength = parser.intValue(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MAX_EXPANSIONS_FIELD)) { - maxExpansions = parser.intValue(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, TRANSPOSITIONS_FIELD)) { - transpositions = parser.booleanValue(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, REWRITE_FIELD)) { - rewrite = parser.textOrNull(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) { - queryName = parser.text(); + String currentFieldName = null; + XContentParser.Token token; + 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(), "[fuzzy] 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 { - throw new ParsingException(parser.getTokenLocation(), "[fuzzy] query does not support [" + currentFieldName + "]"); + if (parseContext.getParseFieldMatcher().match(currentFieldName, TERM_FIELD)) { + value = parser.objectBytes(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, VALUE_FIELD)) { + value = parser.objectBytes(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) { + boost = parser.floatValue(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, Fuzziness.FIELD)) { + fuzziness = Fuzziness.parse(parser); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, PREFIX_LENGTH_FIELD)) { + prefixLength = parser.intValue(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MAX_EXPANSIONS_FIELD)) { + maxExpansions = parser.intValue(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, TRANSPOSITIONS_FIELD)) { + transpositions = parser.booleanValue(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, REWRITE_FIELD)) { + rewrite = parser.textOrNull(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) { + queryName = parser.text(); + } else { + throw new ParsingException(parser.getTokenLocation(), "[fuzzy] query does not support [" + currentFieldName + "]"); + } } } + } else { + fieldName = parser.currentName(); + value = parser.objectBytes(); } - parser.nextToken(); - } else { - value = parser.objectBytes(); - // move to the next token - parser.nextToken(); - } - - if (value == null) { - throw new ParsingException(parser.getTokenLocation(), "no value specified for fuzzy query"); } return Optional.of(new FuzzyQueryBuilder(fieldName, value) .fuzziness(fuzziness) diff --git a/core/src/test/java/org/elasticsearch/index/query/FuzzyQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/FuzzyQueryBuilderTests.java index 92583403638..0330012ddf2 100644 --- a/core/src/test/java/org/elasticsearch/index/query/FuzzyQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/FuzzyQueryBuilderTests.java @@ -23,11 +23,14 @@ import org.apache.lucene.index.Term; import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.FuzzyQuery; import org.apache.lucene.search.Query; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.test.AbstractQueryTestCase; import org.hamcrest.Matchers; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -55,6 +58,19 @@ public class FuzzyQueryBuilderTests extends AbstractQueryTestCase getAlternateVersions() { + Map alternateVersions = new HashMap<>(); + FuzzyQueryBuilder fuzzyQuery = new FuzzyQueryBuilder(randomAsciiOfLengthBetween(1, 10), randomAsciiOfLengthBetween(1, 10)); + String contentString = "{\n" + + " \"fuzzy\" : {\n" + + " \"" + fuzzyQuery.fieldName() + "\" : \"" + fuzzyQuery.value() + "\"\n" + + " }\n" + + "}"; + alternateVersions.put(contentString, fuzzyQuery); + return alternateVersions; + } + @Override protected void doAssertLuceneQuery(FuzzyQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { assertThat(query, instanceOf(FuzzyQuery.class)); @@ -65,21 +81,21 @@ public class FuzzyQueryBuilderTests extends AbstractQueryTestCase