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.
This commit is contained in:
javanna 2016-08-03 20:01:21 +02:00 committed by Luca Cavanna
parent 6d228bb09c
commit 43fee1d7fa
2 changed files with 82 additions and 49 deletions

View File

@ -152,7 +152,7 @@ public class FuzzyQueryBuilder extends AbstractQueryBuilder<FuzzyQueryBuilder> i
*/ */
public FuzzyQueryBuilder(String fieldName, Object value) { public FuzzyQueryBuilder(String fieldName, Object value) {
if (Strings.isEmpty(fieldName)) { 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) { if (value == null) {
throw new IllegalArgumentException("query value cannot be null"); throw new IllegalArgumentException("query value cannot be null");
@ -258,63 +258,59 @@ public class FuzzyQueryBuilder extends AbstractQueryBuilder<FuzzyQueryBuilder> i
public static Optional<FuzzyQueryBuilder> fromXContent(QueryParseContext parseContext) throws IOException { public static Optional<FuzzyQueryBuilder> fromXContent(QueryParseContext parseContext) throws IOException {
XContentParser parser = parseContext.parser(); XContentParser parser = parseContext.parser();
String fieldName = null;
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();
Object value = null; Object value = null;
Fuzziness fuzziness = FuzzyQueryBuilder.DEFAULT_FUZZINESS; Fuzziness fuzziness = FuzzyQueryBuilder.DEFAULT_FUZZINESS;
int prefixLength = FuzzyQueryBuilder.DEFAULT_PREFIX_LENGTH; int prefixLength = FuzzyQueryBuilder.DEFAULT_PREFIX_LENGTH;
int maxExpansions = FuzzyQueryBuilder.DEFAULT_MAX_EXPANSIONS; int maxExpansions = FuzzyQueryBuilder.DEFAULT_MAX_EXPANSIONS;
boolean transpositions = FuzzyQueryBuilder.DEFAULT_TRANSPOSITIONS; boolean transpositions = FuzzyQueryBuilder.DEFAULT_TRANSPOSITIONS;
String rewrite = null; String rewrite = null;
String queryName = null; String queryName = null;
float boost = AbstractQueryBuilder.DEFAULT_BOOST; float boost = AbstractQueryBuilder.DEFAULT_BOOST;
String currentFieldName = null;
token = parser.nextToken(); XContentParser.Token token;
if (token == XContentParser.Token.START_OBJECT) { while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
String currentFieldName = null; if (token == XContentParser.Token.FIELD_NAME) {
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { currentFieldName = parser.currentName();
if (token == XContentParser.Token.FIELD_NAME) { } else if (parseContext.isDeprecatedSetting(currentFieldName)) {
currentFieldName = parser.currentName(); // skip
} else { } else if (token == XContentParser.Token.START_OBJECT) {
if (parseContext.getParseFieldMatcher().match(currentFieldName, TERM_FIELD)) { if (fieldName != null) {
value = parser.objectBytes(); throw new ParsingException(parser.getTokenLocation(), "[fuzzy] query doesn't support multiple fields, found ["
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, VALUE_FIELD)) { + fieldName + "] and [" + currentFieldName + "]");
value = parser.objectBytes(); }
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) { fieldName = currentFieldName;
boost = parser.floatValue(); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, Fuzziness.FIELD)) { if (token == XContentParser.Token.FIELD_NAME) {
fuzziness = Fuzziness.parse(parser); currentFieldName = parser.currentName();
} 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 { } 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) return Optional.of(new FuzzyQueryBuilder(fieldName, value)
.fuzziness(fuzziness) .fuzziness(fuzziness)

View File

@ -23,11 +23,14 @@ import org.apache.lucene.index.Term;
import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.FuzzyQuery; import org.apache.lucene.search.FuzzyQuery;
import org.apache.lucene.search.Query; import org.apache.lucene.search.Query;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.test.AbstractQueryTestCase; import org.elasticsearch.test.AbstractQueryTestCase;
import org.hamcrest.Matchers; import org.hamcrest.Matchers;
import java.io.IOException; import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.instanceOf;
@ -55,6 +58,19 @@ public class FuzzyQueryBuilderTests extends AbstractQueryTestCase<FuzzyQueryBuil
return query; return query;
} }
@Override
protected Map<String, FuzzyQueryBuilder> getAlternateVersions() {
Map<String, FuzzyQueryBuilder> 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 @Override
protected void doAssertLuceneQuery(FuzzyQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { protected void doAssertLuceneQuery(FuzzyQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
assertThat(query, instanceOf(FuzzyQuery.class)); assertThat(query, instanceOf(FuzzyQuery.class));
@ -65,21 +81,21 @@ public class FuzzyQueryBuilderTests extends AbstractQueryTestCase<FuzzyQueryBuil
new FuzzyQueryBuilder(null, "text"); new FuzzyQueryBuilder(null, "text");
fail("must not be null"); fail("must not be null");
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
// expected assertEquals("field name cannot be null or empty", e.getMessage());
} }
try { try {
new FuzzyQueryBuilder("", "text"); new FuzzyQueryBuilder("", "text");
fail("must not be empty"); fail("must not be empty");
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
// expected assertEquals("field name cannot be null or empty", e.getMessage());
} }
try { try {
new FuzzyQueryBuilder("field", null); new FuzzyQueryBuilder("field", null);
fail("must not be null"); fail("must not be null");
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
// expected assertEquals("query value cannot be null", e.getMessage());
} }
} }
@ -157,4 +173,25 @@ public class FuzzyQueryBuilderTests extends AbstractQueryTestCase<FuzzyQueryBuil
assertEquals(json, 42.0, parsed.boost(), 0.00001); assertEquals(json, 42.0, parsed.boost(), 0.00001);
assertEquals(json, 2, parsed.fuzziness().asFloat(), 0f); assertEquals(json, 2, parsed.fuzziness().asFloat(), 0f);
} }
public void testParseFailsWithMultipleFields() throws IOException {
String json = "{\n" +
" \"fuzzy\" : {\n" +
" \"message1\" : {\n" +
" \"value\" : \"this is a test\"\n" +
" },\n" +
" \"message2\" : {\n" +
" \"value\" : \"this is a test\"\n" +
" }\n" +
" }\n" +
"}";
try {
parseQuery(json);
fail("parseQuery should have failed");
} catch(ParsingException e) {
assertEquals("[fuzzy] query doesn't support multiple fields, found [message1] and [message2]", e.getMessage());
}
}
} }