From ce86405394c9b1bb86260f54c019701198430865 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 13 Dec 2016 11:22:15 -0500 Subject: [PATCH] Start to centralize creation of XContentParser in tests (#22096) Starts to centralize creation of the `XContentParser` in `protected final` methods on `ESTestCase`. The idea is to enable adding `NamedXContentRegistry` relatively easily by giving tests a single place they can override to define the `NamedXContentRegistry`. Since `NamedXContentRegistry` doesn't exist yet neither does the override point. This doesn't attempt to migrate all the tests to calling the new methods to build the parsers. I wanted to make this so we could review the concept and then I'll merge a followup to migrate the tests. --- .../common/xcontent/cbor/JsonVsCborTests.java | 3 +- .../xcontent/smile/JsonVsSmileTests.java | 3 +- .../support/XContentMapValuesTests.java | 32 +++++++++---------- .../FunctionScoreQueryBuilderTests.java | 4 +-- .../test/AbstractQueryTestCase.java | 24 +++++++------- .../org/elasticsearch/test/ESTestCase.java | 16 ++++++++++ 6 files changed, 48 insertions(+), 34 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/cbor/JsonVsCborTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/cbor/JsonVsCborTests.java index 8852f090ef6..1b3617bebb4 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/cbor/JsonVsCborTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/cbor/JsonVsCborTests.java @@ -62,8 +62,7 @@ public class JsonVsCborTests extends ESTestCase { xsonGen.close(); jsonGen.close(); - verifySameTokens(XContentFactory.xContent(XContentType.JSON).createParser(jsonOs.bytes()), - XContentFactory.xContent(XContentType.CBOR).createParser(xsonOs.bytes())); + verifySameTokens(createParser(jsonOs.bytes()), createParser(xsonOs.bytes())); } private void verifySameTokens(XContentParser parser1, XContentParser parser2) throws IOException { diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/smile/JsonVsSmileTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/smile/JsonVsSmileTests.java index 9f2910bc11f..0ecb41c78f9 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/smile/JsonVsSmileTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/smile/JsonVsSmileTests.java @@ -62,8 +62,7 @@ public class JsonVsSmileTests extends ESTestCase { xsonGen.close(); jsonGen.close(); - verifySameTokens(XContentFactory.xContent(XContentType.JSON).createParser(jsonOs.bytes()), - XContentFactory.xContent(XContentType.SMILE).createParser(xsonOs.bytes())); + verifySameTokens(createParser(jsonOs.bytes()), createParser(xsonOs.bytes())); } private void verifySameTokens(XContentParser parser1, XContentParser parser2) throws IOException { diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java index 7e757c0bbc4..2a0bf826896 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java @@ -53,7 +53,7 @@ public class XContentMapValuesTests extends ESTestCase { .endObject(); Map source; - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { source = parser.map(); } Map filter = XContentMapValues.filter(source, new String[]{"test1"}, Strings.EMPTY_ARRAY); @@ -81,7 +81,7 @@ public class XContentMapValuesTests extends ESTestCase { .field("test1", "value1") .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { source = parser.map(); } filter = XContentMapValues.filter(source, new String[]{"path1"}, Strings.EMPTY_ARRAY); @@ -107,7 +107,7 @@ public class XContentMapValuesTests extends ESTestCase { .endObject(); Map map; - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } assertThat(XContentMapValues.extractValue("test", map).toString(), equalTo("value")); @@ -118,7 +118,7 @@ public class XContentMapValuesTests extends ESTestCase { .startObject("path1").startObject("path2").field("test", "value").endObject().endObject() .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } assertThat(XContentMapValues.extractValue("path1.path2.test", map).toString(), equalTo("value")); @@ -140,7 +140,7 @@ public class XContentMapValuesTests extends ESTestCase { .startObject("path1").array("test", "value1", "value2").endObject() .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } @@ -159,7 +159,7 @@ public class XContentMapValuesTests extends ESTestCase { .endObject() .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } @@ -175,7 +175,7 @@ public class XContentMapValuesTests extends ESTestCase { builder = XContentFactory.jsonBuilder().startObject() .field("xxx.yyy", "value") .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } assertThat(XContentMapValues.extractValue("xxx.yyy", map).toString(), equalTo("value")); @@ -184,7 +184,7 @@ public class XContentMapValuesTests extends ESTestCase { .startObject("path1.xxx").startObject("path2.yyy").field("test", "value").endObject().endObject() .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } assertThat(XContentMapValues.extractValue("path1.xxx.path2.yyy.test", map).toString(), equalTo("value")); @@ -196,7 +196,7 @@ public class XContentMapValuesTests extends ESTestCase { .endObject(); Map map; - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } assertThat(XContentMapValues.extractRawValues("test", map).get(0).toString(), equalTo("value")); @@ -205,7 +205,7 @@ public class XContentMapValuesTests extends ESTestCase { .field("test.me", "value") .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } assertThat(XContentMapValues.extractRawValues("test.me", map).get(0).toString(), equalTo("value")); @@ -214,7 +214,7 @@ public class XContentMapValuesTests extends ESTestCase { .startObject("path1").startObject("path2").field("test", "value").endObject().endObject() .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } assertThat(XContentMapValues.extractRawValues("path1.path2.test", map).get(0).toString(), equalTo("value")); @@ -223,7 +223,7 @@ public class XContentMapValuesTests extends ESTestCase { .startObject("path1.xxx").startObject("path2.yyy").field("test", "value").endObject().endObject() .endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { map = parser.map(); } assertThat(XContentMapValues.extractRawValues("path1.xxx.path2.yyy.test", map).get(0).toString(), equalTo("value")); @@ -475,7 +475,7 @@ public class XContentMapValuesTests extends ESTestCase { .startArray("some_array") .endArray().endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); assertEquals("some_array", parser.currentName()); @@ -495,7 +495,7 @@ public class XContentMapValuesTests extends ESTestCase { .value(0) .endArray().endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); assertEquals("some_array", parser.currentName()); @@ -515,7 +515,7 @@ public class XContentMapValuesTests extends ESTestCase { .startArray().value(2).endArray() .endArray().endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); assertEquals("some_array", parser.currentName()); @@ -536,7 +536,7 @@ public class XContentMapValuesTests extends ESTestCase { .startObject().endObject() .endArray().endObject(); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(builder.string())) { + try (XContentParser parser = createParser(builder.string())) { assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); assertEquals("some_array", parser.currentName()); diff --git a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index 77edf2ed2f8..7079ccab4cd 100644 --- a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -740,12 +740,12 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase messageMatcher) { + private void expectParsingException(String json, Matcher messageMatcher) { ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(json)); assertThat(e.getMessage(), messageMatcher); } - private static void expectParsingException(String json, String message) { + private void expectParsingException(String json, String message) { expectParsingException(json, equalTo("failed to parse [function_score] query. " + message)); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index aadec9f62a1..6981d564497 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -506,7 +506,7 @@ public abstract class AbstractQueryTestCase> } } - private static void queryWrappedInArrayTest(String queryName, String validQuery) throws IOException { + private void queryWrappedInArrayTest(String queryName, String validQuery) throws IOException { int i = validQuery.indexOf("\"" + queryName + "\""); assertThat(i, greaterThan(0)); @@ -544,11 +544,11 @@ public abstract class AbstractQueryTestCase> /** * Parses the query provided as string argument and compares it with the expected result provided as argument as a {@link QueryBuilder} */ - protected static void assertParsedQuery(String queryAsString, QueryBuilder expectedQuery) throws IOException { + protected void assertParsedQuery(String queryAsString, QueryBuilder expectedQuery) throws IOException { assertParsedQuery(queryAsString, expectedQuery, ParseFieldMatcher.STRICT); } - protected static void assertParsedQuery(String queryAsString, QueryBuilder expectedQuery, ParseFieldMatcher matcher) + protected void assertParsedQuery(String queryAsString, QueryBuilder expectedQuery, ParseFieldMatcher matcher) throws IOException { QueryBuilder newQuery = parseQuery(queryAsString, matcher); assertNotSame(newQuery, expectedQuery); @@ -559,11 +559,11 @@ public abstract class AbstractQueryTestCase> /** * Parses the query provided as bytes argument and compares it with the expected result provided as argument as a {@link QueryBuilder} */ - private static void assertParsedQuery(BytesReference queryAsBytes, QueryBuilder expectedQuery) throws IOException { + private void assertParsedQuery(BytesReference queryAsBytes, QueryBuilder expectedQuery) throws IOException { assertParsedQuery(queryAsBytes, expectedQuery, ParseFieldMatcher.STRICT); } - private static void assertParsedQuery(BytesReference queryAsBytes, QueryBuilder expectedQuery, ParseFieldMatcher matcher) + private void assertParsedQuery(BytesReference queryAsBytes, QueryBuilder expectedQuery, ParseFieldMatcher matcher) throws IOException { QueryBuilder newQuery = parseQuery(queryAsBytes, matcher); assertNotSame(newQuery, expectedQuery); @@ -571,25 +571,25 @@ public abstract class AbstractQueryTestCase> assertEquals(expectedQuery.hashCode(), newQuery.hashCode()); } - protected static QueryBuilder parseQuery(String queryAsString) throws IOException { + protected QueryBuilder parseQuery(String queryAsString) throws IOException { return parseQuery(queryAsString, ParseFieldMatcher.STRICT); } - protected static QueryBuilder parseQuery(String queryAsString, ParseFieldMatcher matcher) throws IOException { - XContentParser parser = XContentFactory.xContent(queryAsString).createParser(queryAsString); + protected QueryBuilder parseQuery(String queryAsString, ParseFieldMatcher matcher) throws IOException { + XContentParser parser = createParser(queryAsString); return parseQuery(parser, matcher); } - protected static QueryBuilder parseQuery(BytesReference queryAsBytes) throws IOException { + protected QueryBuilder parseQuery(BytesReference queryAsBytes) throws IOException { return parseQuery(queryAsBytes, ParseFieldMatcher.STRICT); } - protected static QueryBuilder parseQuery(BytesReference queryAsBytes, ParseFieldMatcher matcher) throws IOException { - XContentParser parser = XContentFactory.xContent(queryAsBytes).createParser(queryAsBytes); + protected QueryBuilder parseQuery(BytesReference queryAsBytes, ParseFieldMatcher matcher) throws IOException { + XContentParser parser = createParser(queryAsBytes); return parseQuery(parser, matcher); } - private static QueryBuilder parseQuery(XContentParser parser, ParseFieldMatcher matcher) throws IOException { + private QueryBuilder parseQuery(XContentParser parser, ParseFieldMatcher matcher) throws IOException { QueryParseContext context = createParseContext(parser, matcher); QueryBuilder parseInnerQueryBuilder = context.parseInnerQueryBuilder(); assertNull(parser.nextToken()); diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index b98a305c7cf..4bf47228f47 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -47,6 +47,7 @@ import org.elasticsearch.Version; import org.elasticsearch.bootstrap.BootstrapForTesting; import org.elasticsearch.client.Requests; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.io.PathUtilsForTesting; @@ -63,6 +64,7 @@ import org.elasticsearch.common.util.MockBigArrays; import org.elasticsearch.common.util.MockPageCacheRecycler; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; @@ -878,6 +880,20 @@ public abstract class ESTestCase extends LuceneTestCase { assertThat(count + " files exist that should have been cleaned:\n" + sb.toString(), count, equalTo(0)); } + /** + * Create a new {@link XContentParser}. + */ + protected final XContentParser createParser(BytesReference data) throws IOException { + return XContentFactory.xContent(data).createParser(data); + } + + /** + * Create a new {@link XContentParser}. + */ + protected final XContentParser createParser(String string) throws IOException { + return createParser(new BytesArray(string)); + } + /** Returns the suite failure marker: internal use only! */ public static TestRuleMarkFailure getSuiteFailureMarker() { return suiteFailureMarker;