Throw parsing error if wildcard query contains multiple fields

Wildcard 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 18:44:23 +02:00 committed by Luca Cavanna
parent 003a7b6eb3
commit 51ea913248
2 changed files with 85 additions and 45 deletions

View File

@ -75,7 +75,7 @@ public class WildcardQueryBuilder extends AbstractQueryBuilder<WildcardQueryBuil
throw new IllegalArgumentException("field name is null or empty"); throw new IllegalArgumentException("field name is null or empty");
} }
if (value == null) { if (value == null) {
throw new IllegalArgumentException("value cannot be null."); throw new IllegalArgumentException("value cannot be null");
} }
this.fieldName = fieldName; this.fieldName = fieldName;
this.value = value; this.value = value;
@ -135,49 +135,50 @@ public class WildcardQueryBuilder extends AbstractQueryBuilder<WildcardQueryBuil
public static Optional<WildcardQueryBuilder> fromXContent(QueryParseContext parseContext) throws IOException { public static Optional<WildcardQueryBuilder> 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(), "[wildcard] query malformed, no field");
}
String fieldName = parser.currentName();
String rewrite = null; String rewrite = null;
String value = null; String value = null;
float boost = AbstractQueryBuilder.DEFAULT_BOOST; float boost = AbstractQueryBuilder.DEFAULT_BOOST;
String queryName = null; String queryName = null;
token = parser.nextToken(); String currentFieldName = null;
if (token == XContentParser.Token.START_OBJECT) { XContentParser.Token token;
String currentFieldName = null; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) {
if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName();
currentFieldName = parser.currentName(); } else if (parseContext.isDeprecatedSetting(currentFieldName)) {
} else { // skip
if (parseContext.getParseFieldMatcher().match(currentFieldName, WILDCARD_FIELD)) { } else if (token == XContentParser.Token.START_OBJECT) {
value = parser.text(); if (fieldName != null) {
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, VALUE_FIELD)) { throw new ParsingException(parser.getTokenLocation(), "[wildcard] query doesn't support multiple fields, found ["
value = parser.text(); + fieldName + "] and [" + currentFieldName + "]");
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) { }
boost = parser.floatValue(); fieldName = currentFieldName;
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, REWRITE_FIELD)) { while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
rewrite = parser.textOrNull(); if (token == XContentParser.Token.FIELD_NAME) {
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) { currentFieldName = parser.currentName();
queryName = parser.text();
} else { } else {
throw new ParsingException(parser.getTokenLocation(), if (parseContext.getParseFieldMatcher().match(currentFieldName, WILDCARD_FIELD)) {
"[wildcard] query does not support [" + currentFieldName + "]"); value = parser.text();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, VALUE_FIELD)) {
value = parser.text();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) {
boost = parser.floatValue();
} 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(),
"[wildcard] query does not support [" + currentFieldName + "]");
}
} }
} }
} else {
fieldName = parser.currentName();
value = parser.text();
} }
parser.nextToken();
} else {
value = parser.text();
parser.nextToken();
} }
if (value == null) {
throw new ParsingException(parser.getTokenLocation(), "No value specified for wildcard query");
}
return Optional.of(new WildcardQueryBuilder(fieldName, value) return Optional.of(new WildcardQueryBuilder(fieldName, value)
.rewrite(rewrite) .rewrite(rewrite)
.boost(boost) .boost(boost)

View File

@ -21,9 +21,12 @@ package org.elasticsearch.index.query;
import org.apache.lucene.search.Query; import org.apache.lucene.search.Query;
import org.apache.lucene.search.WildcardQuery; import org.apache.lucene.search.WildcardQuery;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.test.AbstractQueryTestCase; import org.elasticsearch.test.AbstractQueryTestCase;
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;
@ -32,21 +35,36 @@ public class WildcardQueryBuilderTests extends AbstractQueryTestCase<WildcardQue
@Override @Override
protected WildcardQueryBuilder doCreateTestQueryBuilder() { protected WildcardQueryBuilder doCreateTestQueryBuilder() {
WildcardQueryBuilder query; WildcardQueryBuilder query = randomWildcardQuery();
// mapped or unmapped field
String text = randomAsciiOfLengthBetween(1, 10);
if (randomBoolean()) {
query = new WildcardQueryBuilder(STRING_FIELD_NAME, text);
} else {
query = new WildcardQueryBuilder(randomAsciiOfLengthBetween(1, 10), text);
}
if (randomBoolean()) { if (randomBoolean()) {
query.rewrite(randomFrom(getRandomRewriteMethod())); query.rewrite(randomFrom(getRandomRewriteMethod()));
} }
return query; return query;
} }
@Override
protected Map<String, WildcardQueryBuilder> getAlternateVersions() {
Map<String, WildcardQueryBuilder> alternateVersions = new HashMap<>();
WildcardQueryBuilder wildcardQuery = randomWildcardQuery();
String contentString = "{\n" +
" \"wildcard\" : {\n" +
" \"" + wildcardQuery.fieldName() + "\" : \"" + wildcardQuery.value() + "\"\n" +
" }\n" +
"}";
alternateVersions.put(contentString, wildcardQuery);
return alternateVersions;
}
private static WildcardQueryBuilder randomWildcardQuery() {
// mapped or unmapped field
String text = randomAsciiOfLengthBetween(1, 10);
if (randomBoolean()) {
return new WildcardQueryBuilder(STRING_FIELD_NAME, text);
} else {
return new WildcardQueryBuilder(randomAsciiOfLengthBetween(1, 10), text);
}
}
@Override @Override
protected void doAssertLuceneQuery(WildcardQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { protected void doAssertLuceneQuery(WildcardQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
assertThat(query, instanceOf(WildcardQuery.class)); assertThat(query, instanceOf(WildcardQuery.class));
@ -65,14 +83,14 @@ public class WildcardQueryBuilderTests extends AbstractQueryTestCase<WildcardQue
} }
fail("cannot be null or empty"); fail("cannot be null or empty");
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
// expected assertEquals("field name is null or empty", e.getMessage());
} }
try { try {
new WildcardQueryBuilder("field", null); new WildcardQueryBuilder("field", null);
fail("cannot be null or empty"); fail("cannot be null or empty");
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
// expected assertEquals("value cannot be null", e.getMessage());
} }
} }
@ -94,4 +112,25 @@ public class WildcardQueryBuilderTests extends AbstractQueryTestCase<WildcardQue
assertEquals(json, "ki*y", parsed.value()); assertEquals(json, "ki*y", parsed.value());
assertEquals(json, 2.0, parsed.boost(), 0.0001); assertEquals(json, 2.0, parsed.boost(), 0.0001);
} }
public void testParseFailsWithMultipleFields() throws IOException {
String json =
"{\n" +
" \"wildcard\": {\n" +
" \"user1\": {\n" +
" \"wildcard\": \"ki*y\"\n" +
" },\n" +
" \"user2\": {\n" +
" \"wildcard\": \"ki*y\"\n" +
" }\n" +
" }\n" +
"}";
try {
parseQuery(json);
fail("parseQuery should have failed");
} catch(ParsingException e) {
assertEquals("[wildcard] query doesn't support multiple fields, found [user1] and [user2]", e.getMessage());
}
}
} }