Throw parsing error if common terms query contains multiple fields
Common Terms 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:
parent
1e45fd5850
commit
c3dfe0846c
|
@ -102,7 +102,7 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder<CommonTermsQue
|
|||
throw new IllegalArgumentException("field name is null or empty");
|
||||
}
|
||||
if (text == null) {
|
||||
throw new IllegalArgumentException("text cannot be null.");
|
||||
throw new IllegalArgumentException("text cannot be null");
|
||||
}
|
||||
this.fieldName = fieldName;
|
||||
this.text = text;
|
||||
|
@ -265,11 +265,8 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder<CommonTermsQue
|
|||
|
||||
public static Optional<CommonTermsQueryBuilder> 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 text = null;
|
||||
float boost = AbstractQueryBuilder.DEFAULT_BOOST;
|
||||
String analyzer = null;
|
||||
|
@ -280,9 +277,19 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder<CommonTermsQue
|
|||
Operator lowFreqOperator = CommonTermsQueryBuilder.DEFAULT_LOW_FREQ_OCCUR;
|
||||
float cutoffFrequency = CommonTermsQueryBuilder.DEFAULT_CUTOFF_FREQ;
|
||||
String queryName = null;
|
||||
token = parser.nextToken();
|
||||
if (token == XContentParser.Token.START_OBJECT) {
|
||||
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(), "[common] 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();
|
||||
|
@ -337,21 +344,12 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder<CommonTermsQue
|
|||
}
|
||||
}
|
||||
}
|
||||
parser.nextToken();
|
||||
} else {
|
||||
fieldName = parser.currentName();
|
||||
text = parser.objectText();
|
||||
// move to the next token
|
||||
token = parser.nextToken();
|
||||
if (token != XContentParser.Token.END_OBJECT) {
|
||||
throw new ParsingException(parser.getTokenLocation(),
|
||||
"[common] 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 (text == null) {
|
||||
throw new ParsingException(parser.getTokenLocation(), "No text specified for text query");
|
||||
}
|
||||
return Optional.of(new CommonTermsQueryBuilder(fieldName, text)
|
||||
.lowFreqMinimumShouldMatch(lowFreqMinimumShouldMatch)
|
||||
.highFreqMinimumShouldMatch(highFreqMinimumShouldMatch)
|
||||
|
|
|
@ -21,9 +21,12 @@ package org.elasticsearch.index.query;
|
|||
|
||||
import org.apache.lucene.queries.ExtendedCommonTermsQuery;
|
||||
import org.apache.lucene.search.Query;
|
||||
import org.elasticsearch.common.ParsingException;
|
||||
import org.elasticsearch.test.AbstractQueryTestCase;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static org.elasticsearch.index.query.QueryBuilders.commonTermsQuery;
|
||||
import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath;
|
||||
|
@ -81,6 +84,20 @@ public class CommonTermsQueryBuilderTests extends AbstractQueryTestCase<CommonTe
|
|||
return query;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Map<String, CommonTermsQueryBuilder> getAlternateVersions() {
|
||||
Map<String, CommonTermsQueryBuilder> alternateVersions = new HashMap<>();
|
||||
CommonTermsQueryBuilder commonTermsQuery = new CommonTermsQueryBuilder(randomAsciiOfLengthBetween(1, 10),
|
||||
randomAsciiOfLengthBetween(1, 10));
|
||||
String contentString = "{\n" +
|
||||
" \"common\" : {\n" +
|
||||
" \"" + commonTermsQuery.fieldName() + "\" : \"" + commonTermsQuery.value() + "\"\n" +
|
||||
" }\n" +
|
||||
"}";
|
||||
alternateVersions.put(contentString, commonTermsQuery);
|
||||
return alternateVersions;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(CommonTermsQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
assertThat(query, instanceOf(ExtendedCommonTermsQuery.class));
|
||||
|
@ -98,14 +115,14 @@ public class CommonTermsQueryBuilderTests extends AbstractQueryTestCase<CommonTe
|
|||
}
|
||||
fail("must be non null");
|
||||
} catch (IllegalArgumentException e) {
|
||||
// okay
|
||||
assertEquals("field name is null or empty", e.getMessage());
|
||||
}
|
||||
|
||||
try {
|
||||
new CommonTermsQueryBuilder("fieldName", null);
|
||||
fail("must be non null");
|
||||
} catch (IllegalArgumentException e) {
|
||||
// okay
|
||||
assertEquals("text cannot be null", e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -173,4 +190,24 @@ public class CommonTermsQueryBuilderTests extends AbstractQueryTestCase<CommonTe
|
|||
ExtendedCommonTermsQuery ectQuery = (ExtendedCommonTermsQuery) parsedQuery;
|
||||
assertThat(ectQuery.isCoordDisabled(), equalTo(disableCoord));
|
||||
}
|
||||
|
||||
public void testParseFailsWithMultipleFields() throws IOException {
|
||||
String json = "{\n" +
|
||||
" \"common\" : {\n" +
|
||||
" \"message1\" : {\n" +
|
||||
" \"query\" : \"nelly the elephant not as a cartoon\"\n" +
|
||||
" },\n" +
|
||||
" \"message2\" : {\n" +
|
||||
" \"query\" : \"nelly the elephant not as a cartoon\"\n" +
|
||||
" }\n" +
|
||||
" }\n" +
|
||||
"}";
|
||||
|
||||
try {
|
||||
parseQuery(json);
|
||||
fail("parseQuery should have failed");
|
||||
} catch(ParsingException e) {
|
||||
assertEquals("[common] query doesn't support multiple fields, found [message1] and [message2]", e.getMessage());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue