Skip to content

Commit dfebbbf

Browse files
romseygeekromseygeek
committed
BoolQueryBuilder uses ObjectParser (#52880)
This commit removes the hand-rolled x-content parsing logic from BoolQueryBuilder and instead uses an ObjectParser to handle parsing. It also removes the long-deprecated (since version 6) disable_coord parameter.
1 parent 812d981 commit dfebbbf

File tree

2 files changed

+41
-99
lines changed

2 files changed

+41
-99
lines changed

‎server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java‎

Lines changed: 28 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.elasticsearch.common.io.stream.StreamInput;
3131
import org.elasticsearch.common.io.stream.StreamOutput;
3232
import org.elasticsearch.common.lucene.search.Queries;
33+
import org.elasticsearch.common.xcontent.ObjectParser;
3334
import org.elasticsearch.common.xcontent.XContentBuilder;
3435
import org.elasticsearch.common.xcontent.XContentParser;
3536

@@ -52,13 +53,11 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> {
5253

5354
public static final boolean ADJUST_PURE_NEGATIVE_DEFAULT = true;
5455

55-
private static final String MUSTNOT = "mustNot";
56-
private static final String MUST_NOT = "must_not";
57-
private static final String FILTER = "filter";
58-
private static final String SHOULD = "should";
59-
private static final String MUST = "must";
60-
private static final ParseField DISABLE_COORD_FIELD = new ParseField("disable_coord")
61-
.withAllDeprecated("disable_coord has been removed");
56+
private static final ParseField MUSTNOT = new ParseField("mustNot"); // TODO deprecate?
57+
private static final ParseField MUST_NOT = new ParseField("must_not");
58+
private static final ParseField FILTER = new ParseField("filter");
59+
private static final ParseField SHOULD = new ParseField("should");
60+
private static final ParseField MUST = new ParseField("must");
6261
private static final ParseField MINIMUM_SHOULD_MATCH = new ParseField("minimum_should_match");
6362
private static final ParseField ADJUST_PURE_NEGATIVE = new ParseField("adjust_pure_negative");
6463

@@ -265,106 +264,39 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
265264
builder.endObject();
266265
}
267266

268-
private static void doXArrayContent(String field, List<QueryBuilder> clauses, XContentBuilder builder, Params params)
267+
private static void doXArrayContent(ParseField field, List<QueryBuilder> clauses, XContentBuilder builder, Params params)
269268
throws IOException {
270269
if (clauses.isEmpty()) {
271270
return;
272271
}
273-
builder.startArray(field);
272+
builder.startArray(field.getPreferredName());
274273
for (QueryBuilder clause : clauses) {
275274
clause.toXContent(builder, params);
276275
}
277276
builder.endArray();
278277
}
279278

279+
private static final ObjectParser<BoolQueryBuilder, Void> PARSER = new ObjectParser<>("bool", BoolQueryBuilder::new);
280+
static {
281+
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::must), (p, c) -> parseInnerQueryBuilder(p),
282+
MUST);
283+
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::should), (p, c) -> parseInnerQueryBuilder(p),
284+
SHOULD);
285+
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::mustNot), (p, c) -> parseInnerQueryBuilder(p),
286+
MUST_NOT);
287+
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::mustNot), (p, c) -> parseInnerQueryBuilder(p),
288+
MUSTNOT); // TODO should we deprecate this version?
289+
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::filter), (p, c) -> parseInnerQueryBuilder(p),
290+
FILTER);
291+
PARSER.declareBoolean(BoolQueryBuilder::adjustPureNegative, ADJUST_PURE_NEGATIVE);
292+
PARSER.declareField(BoolQueryBuilder::minimumShouldMatch, (p, c) -> p.text(),
293+
MINIMUM_SHOULD_MATCH, ObjectParser.ValueType.VALUE);
294+
PARSER.declareString(BoolQueryBuilder::queryName, NAME_FIELD);
295+
PARSER.declareFloat(BoolQueryBuilder::boost, BOOST_FIELD);
296+
}
297+
280298
public static BoolQueryBuilder fromXContent(XContentParser parser) throws IOException, ParsingException {
281-
boolean adjustPureNegative = BoolQueryBuilder.ADJUST_PURE_NEGATIVE_DEFAULT;
282-
float boost = DEFAULT_BOOST;
283-
String minimumShouldMatch = null;
284-
285-
final List<QueryBuilder> mustClauses = new ArrayList<>();
286-
final List<QueryBuilder> mustNotClauses = new ArrayList<>();
287-
final List<QueryBuilder> shouldClauses = new ArrayList<>();
288-
final List<QueryBuilder> filterClauses = new ArrayList<>();
289-
String queryName = null;
290-
291-
String currentFieldName = null;
292-
XContentParser.Token token;
293-
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
294-
if (token == XContentParser.Token.FIELD_NAME) {
295-
currentFieldName = parser.currentName();
296-
} else if (token == XContentParser.Token.START_OBJECT) {
297-
switch (currentFieldName) {
298-
case MUST:
299-
mustClauses.add(parseInnerQueryBuilder(parser));
300-
break;
301-
case SHOULD:
302-
shouldClauses.add(parseInnerQueryBuilder(parser));
303-
break;
304-
case FILTER:
305-
filterClauses.add(parseInnerQueryBuilder(parser));
306-
break;
307-
case MUST_NOT:
308-
case MUSTNOT:
309-
mustNotClauses.add(parseInnerQueryBuilder(parser));
310-
break;
311-
default:
312-
throw new ParsingException(parser.getTokenLocation(), "[bool] query does not support [" + currentFieldName + "]");
313-
}
314-
} else if (token == XContentParser.Token.START_ARRAY) {
315-
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
316-
switch (currentFieldName) {
317-
case MUST:
318-
mustClauses.add(parseInnerQueryBuilder(parser));
319-
break;
320-
case SHOULD:
321-
shouldClauses.add(parseInnerQueryBuilder(parser));
322-
break;
323-
case FILTER:
324-
filterClauses.add(parseInnerQueryBuilder(parser));
325-
break;
326-
case MUST_NOT:
327-
case MUSTNOT:
328-
mustNotClauses.add(parseInnerQueryBuilder(parser));
329-
break;
330-
default:
331-
throw new ParsingException(parser.getTokenLocation(), "bool query does not support [" + currentFieldName + "]");
332-
}
333-
}
334-
} else if (token.isValue()) {
335-
if (DISABLE_COORD_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
336-
// ignore
337-
} else if (MINIMUM_SHOULD_MATCH.match(currentFieldName, parser.getDeprecationHandler())) {
338-
minimumShouldMatch = parser.textOrNull();
339-
} else if (BOOST_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
340-
boost = parser.floatValue();
341-
} else if (ADJUST_PURE_NEGATIVE.match(currentFieldName, parser.getDeprecationHandler())) {
342-
adjustPureNegative = parser.booleanValue();
343-
} else if (NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
344-
queryName = parser.text();
345-
} else {
346-
throw new ParsingException(parser.getTokenLocation(), "[bool] query does not support [" + currentFieldName + "]");
347-
}
348-
}
349-
}
350-
BoolQueryBuilder boolQuery = new BoolQueryBuilder();
351-
for (QueryBuilder queryBuilder : mustClauses) {
352-
boolQuery.must(queryBuilder);
353-
}
354-
for (QueryBuilder queryBuilder : mustNotClauses) {
355-
boolQuery.mustNot(queryBuilder);
356-
}
357-
for (QueryBuilder queryBuilder : shouldClauses) {
358-
boolQuery.should(queryBuilder);
359-
}
360-
for (QueryBuilder queryBuilder : filterClauses) {
361-
boolQuery.filter(queryBuilder);
362-
}
363-
boolQuery.boost(boost);
364-
boolQuery.adjustPureNegative(adjustPureNegative);
365-
boolQuery.minimumShouldMatch(minimumShouldMatch);
366-
boolQuery.queryName(queryName);
367-
return boolQuery;
299+
return PARSER.parse(parser, null);
368300
}
369301

370302
@Override

‎server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
import org.apache.lucene.search.MatchAllDocsQuery;
2525
import org.apache.lucene.search.MatchNoDocsQuery;
2626
import org.apache.lucene.search.Query;
27-
import org.elasticsearch.common.ParsingException;
2827
import org.elasticsearch.common.xcontent.XContentBuilder;
2928
import org.elasticsearch.common.xcontent.XContentFactory;
29+
import org.elasticsearch.common.xcontent.XContentParseException;
3030
import org.elasticsearch.common.xcontent.XContentParser;
3131
import org.elasticsearch.common.xcontent.XContentType;
3232
import org.elasticsearch.test.AbstractQueryTestCase;
@@ -41,6 +41,7 @@
4141

4242
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
4343
import static org.elasticsearch.index.query.QueryBuilders.termQuery;
44+
import static org.hamcrest.CoreMatchers.containsString;
4445
import static org.hamcrest.CoreMatchers.equalTo;
4546
import static org.hamcrest.CoreMatchers.instanceOf;
4647

@@ -278,14 +279,23 @@ public void testFromJson() throws IOException {
278279
assertEquals(query, "kimchy", ((TermQueryBuilder)queryBuilder.must().get(0)).value());
279280
}
280281

282+
public void testMinimumShouldMatchNumber() throws IOException {
283+
String query = "{\"bool\" : {\"must\" : { \"term\" : { \"field\" : \"value\" } }, \"minimum_should_match\" : 1 } }";
284+
BoolQueryBuilder builder = (BoolQueryBuilder) parseQuery(query);
285+
assertEquals("1", builder.minimumShouldMatch());
286+
}
287+
281288
/**
282289
* test that unknown query names in the clauses throw an error
283290
*/
284291
public void testUnknownQueryName() throws IOException {
285292
String query = "{\"bool\" : {\"must\" : { \"unknown_query\" : { } } } }";
286293

287-
ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query));
288-
assertEquals("unknown query [unknown_query]", ex.getMessage());
294+
XContentParseException ex = expectThrows(XContentParseException.class, () -> parseQuery(query));
295+
assertEquals("[1:41] [bool] failed to parse field [must]", ex.getMessage());
296+
Throwable e = ex.getCause();
297+
assertThat(e.getMessage(), containsString("unknown query [unknown_query]"));
298+
289299
}
290300

291301
public void testRewrite() throws IOException {

0 commit comments

Comments
 (0)