QueryBuilders should implement ToXContentObject (#25530)

All query builders written as self contained xContent objects, to we should mark
them accordingly using ToXContentObject. This also makes it possible to use
things like XContentHelper#toXContent to render query builders in tests.
This commit is contained in:
Christoph Büscher 2017-07-05 09:50:10 +02:00 committed by GitHub
parent 3da3632021
commit 3185eaece8
5 changed files with 14 additions and 23 deletions

View File

@ -311,7 +311,6 @@ public abstract class AbstractQueryBuilder<QB extends AbstractQueryBuilder<QB>>
}
QueryBuilder result;
try {
// TODO what can we pass in here
result = parser.namedObject(QueryBuilder.class, queryName, null);
} catch (UnknownNamedObjectException e) {
// Preserve the error message from 5.0 until we have a compellingly better message so we don't break BWC.

View File

@ -21,11 +21,11 @@ package org.elasticsearch.index.query;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.io.stream.NamedWriteable;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import java.io.IOException;
public interface QueryBuilder extends NamedWriteable, ToXContent {
public interface QueryBuilder extends NamedWriteable, ToXContentObject {
/**
* Converts this QueryBuilder to a lucene {@link Query}.

View File

@ -23,7 +23,6 @@ import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.AbstractQueryTestCase;
@ -94,7 +93,7 @@ public class ConstantScoreQueryBuilderTests extends AbstractQueryTestCase<Consta
}
@Override
public void testUnknownField() throws IOException {
public void testUnknownField() {
assumeTrue("test doesn't apply for query filter queries", false);
}

View File

@ -78,7 +78,7 @@ public class WrapperQueryBuilderTests extends AbstractQueryTestCase<WrapperQuery
* anything else.
*/
@Override
public void testUnknownField() throws IOException {
public void testUnknownField() {
String json = "{ \"" + WrapperQueryBuilder.NAME + "\" : {\"bogusField\" : \"someValue\"} }";
ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(json));
assertTrue(e.getMessage().contains("bogusField"));

View File

@ -40,6 +40,7 @@ import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
@ -226,7 +227,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
}
@After
public void afterTest() throws IOException {
public void afterTest() {
serviceHolder.clientInvocationHandler.delegate = null;
}
@ -265,9 +266,10 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
public void testFromXContent() throws IOException {
for (int runs = 0; runs < NUMBER_OF_TESTQUERIES; runs++) {
QB testQuery = createTestQueryBuilder();
XContentBuilder builder = toXContent(testQuery, randomFrom(XContentType.values()));
XContentBuilder shuffled = shuffleXContent(builder, shuffleProtectedFields());
assertParsedQuery(createParser(shuffled), testQuery);
XContentType xContentType = randomFrom(XContentType.values());
BytesReference shuffledXContent = toShuffledXContent(testQuery, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean(),
shuffleProtectedFields());
assertParsedQuery(createParser(xContentType.xContent(), shuffledXContent), testQuery);
for (Map.Entry<String, QB> alternateVersion : getAlternateVersions().entrySet()) {
String queryAsString = alternateVersion.getKey();
assertParsedQuery(createParser(JsonXContent.jsonXContent, queryAsString), alternateVersion.getValue());
@ -283,22 +285,13 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
return Strings.EMPTY_ARRAY;
}
protected static XContentBuilder toXContent(QueryBuilder query, XContentType contentType) throws IOException {
XContentBuilder builder = XContentFactory.contentBuilder(contentType);
if (randomBoolean()) {
builder.prettyPrint();
}
query.toXContent(builder, ToXContent.EMPTY_PARAMS);
return builder;
}
/**
* Test that unknown field trigger ParsingException.
* To find the right position in the root query, we add a marker as `queryName` which
* all query builders support. The added bogus field after that should trigger the exception.
* Queries that allow arbitrary field names at this level need to override this test.
*/
public void testUnknownField() throws IOException {
public void testUnknownField() {
String marker = "#marker#";
QB testQuery;
do {
@ -471,7 +464,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
* Test that wraps the randomly generated query into an array as follows: { "query_name" : [{}]}
* This causes unexpected situations in parser code that may not be handled properly.
*/
public final void testQueryWrappedInArray() throws IOException {
public final void testQueryWrappedInArray() {
QB queryBuilder = createTestQueryBuilder();
String queryName = queryBuilder.getName();
String validQuery = queryBuilder.toString();
@ -481,7 +474,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
}
}
private void queryWrappedInArrayTest(String queryName, String validQuery) throws IOException {
private void queryWrappedInArrayTest(String queryName, String validQuery) {
int i = validQuery.indexOf("\"" + queryName + "\"");
assertThat(i, greaterThan(0));
@ -727,7 +720,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
}
}
public void testEqualsAndHashcode() throws IOException {
public void testEqualsAndHashcode() {
for (int runs = 0; runs < NUMBER_OF_TESTQUERIES; runs++) {
// TODO we only change name and boost, we should extend by any sub-test supplying a "mutate" method that randomly changes one
// aspect of the object under test