diff options
7 files changed, 97 insertions, 38 deletions
diff --git a/core/src/main/java/org/elasticsearch/search/profile/ProfileResult.java b/core/src/main/java/org/elasticsearch/search/profile/ProfileResult.java index 3b1bfe3c27..16a2f8c8eb 100644 --- a/core/src/main/java/org/elasticsearch/search/profile/ProfileResult.java +++ b/core/src/main/java/org/elasticsearch/search/profile/ProfileResult.java @@ -37,7 +37,6 @@ import java.util.Objects; import java.util.concurrent.TimeUnit; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField; /** * This class is the internal representation of a profiled Query, corresponding @@ -50,12 +49,12 @@ import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknown */ public final class ProfileResult implements Writeable, ToXContentObject { - private static final ParseField TYPE = new ParseField("type"); - private static final ParseField DESCRIPTION = new ParseField("description"); - private static final ParseField NODE_TIME = new ParseField("time"); - private static final ParseField NODE_TIME_RAW = new ParseField("time_in_nanos"); - private static final ParseField CHILDREN = new ParseField("children"); - private static final ParseField BREAKDOWN = new ParseField("breakdown"); + static final ParseField TYPE = new ParseField("type"); + static final ParseField DESCRIPTION = new ParseField("description"); + static final ParseField NODE_TIME = new ParseField("time"); + static final ParseField NODE_TIME_RAW = new ParseField("time_in_nanos"); + static final ParseField CHILDREN = new ParseField("children"); + static final ParseField BREAKDOWN = new ParseField("breakdown"); private final String type; private final String description; @@ -188,7 +187,7 @@ public final class ProfileResult implements Writeable, ToXContentObject { // skip, total time is calculate by adding up 'timings' values in ProfileResult ctor parser.longValue(); } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } else if (token == XContentParser.Token.START_OBJECT) { if (BREAKDOWN.match(currentFieldName)) { @@ -200,7 +199,7 @@ public final class ProfileResult implements Writeable, ToXContentObject { timings.put(name, value); } } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } else if (token == XContentParser.Token.START_ARRAY) { if (CHILDREN.match(currentFieldName)) { @@ -208,7 +207,7 @@ public final class ProfileResult implements Writeable, ToXContentObject { children.add(ProfileResult.fromXContent(parser)); } } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } } diff --git a/core/src/main/java/org/elasticsearch/search/profile/SearchProfileShardResults.java b/core/src/main/java/org/elasticsearch/search/profile/SearchProfileShardResults.java index eb3017bd1e..b7fa39c42f 100644 --- a/core/src/main/java/org/elasticsearch/search/profile/SearchProfileShardResults.java +++ b/core/src/main/java/org/elasticsearch/search/profile/SearchProfileShardResults.java @@ -39,9 +39,6 @@ import java.util.Map; import java.util.TreeSet; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownToken; /** * A container class to hold all the profile results across all shards. Internally @@ -111,12 +108,19 @@ public final class SearchProfileShardResults implements Writeable, ToXContent{ XContentParser.Token token = parser.currentToken(); ensureExpectedToken(XContentParser.Token.START_OBJECT, token, parser::getTokenLocation); Map<String, ProfileShardResult> searchProfileResults = new HashMap<>(); - ensureFieldName(parser, parser.nextToken(), SHARDS_FIELD); - ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.nextToken(), parser::getTokenLocation); - while((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - parseSearchProfileResultsEntry(parser, searchProfileResults); + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.START_ARRAY) { + if (SHARDS_FIELD.equals(parser.currentName())) { + while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + parseSearchProfileResultsEntry(parser, searchProfileResults); + } + } else { + parser.skipChildren(); + } + } else if (token == XContentParser.Token.START_OBJECT) { + parser.skipChildren(); + } } - ensureExpectedToken(XContentParser.Token.END_OBJECT, parser.nextToken(), parser::getTokenLocation); return new SearchProfileShardResults(searchProfileResults); } @@ -135,7 +139,7 @@ public final class SearchProfileShardResults implements Writeable, ToXContent{ if (ID_FIELD.equals(currentFieldName)) { id = parser.text(); } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } else if (token == XContentParser.Token.START_ARRAY) { if (SEARCHES_FIELD.equals(currentFieldName)) { @@ -145,10 +149,10 @@ public final class SearchProfileShardResults implements Writeable, ToXContent{ } else if (AggregationProfileShardResult.AGGREGATIONS.equals(currentFieldName)) { aggProfileShardResult = AggregationProfileShardResult.fromXContent(parser); } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } else { - throwUnknownToken(token, parser.getTokenLocation()); + parser.skipChildren(); } } searchProfileResults.put(id, new ProfileShardResult(queryProfileResults, aggProfileShardResult)); diff --git a/core/src/main/java/org/elasticsearch/search/profile/query/CollectorResult.java b/core/src/main/java/org/elasticsearch/search/profile/query/CollectorResult.java index 1fa56bde7f..0d4ae0384b 100644 --- a/core/src/main/java/org/elasticsearch/search/profile/query/CollectorResult.java +++ b/core/src/main/java/org/elasticsearch/search/profile/query/CollectorResult.java @@ -34,8 +34,6 @@ import java.util.List; import java.util.concurrent.TimeUnit; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownToken; /** * Public interface and serialization container for profiled timings of the @@ -181,7 +179,7 @@ public class CollectorResult implements ToXContentObject, Writeable { } else if (TIME_NANOS.match(currentFieldName)) { time = parser.longValue(); } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } else if (token == XContentParser.Token.START_ARRAY) { if (CHILDREN.match(currentFieldName)) { @@ -189,10 +187,10 @@ public class CollectorResult implements ToXContentObject, Writeable { children.add(CollectorResult.fromXContent(parser)); } } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } else { - throwUnknownToken(token, parser.getTokenLocation()); + parser.skipChildren(); } } return new CollectorResult(name, reason, time, children); diff --git a/core/src/main/java/org/elasticsearch/search/profile/query/QueryProfileShardResult.java b/core/src/main/java/org/elasticsearch/search/profile/query/QueryProfileShardResult.java index 362826e860..81062ed75c 100644 --- a/core/src/main/java/org/elasticsearch/search/profile/query/QueryProfileShardResult.java +++ b/core/src/main/java/org/elasticsearch/search/profile/query/QueryProfileShardResult.java @@ -33,8 +33,6 @@ import java.util.Collections; import java.util.List; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownToken; /** * A container class to hold the profile results for a single shard in the request. @@ -127,7 +125,7 @@ public final class QueryProfileShardResult implements Writeable, ToXContentObjec if (REWRITE_TIME.equals(currentFieldName)) { rewriteTime = parser.longValue(); } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } else if (token == XContentParser.Token.START_ARRAY) { if (QUERY_ARRAY.equals(currentFieldName)) { @@ -139,10 +137,10 @@ public final class QueryProfileShardResult implements Writeable, ToXContentObjec collector = CollectorResult.fromXContent(parser); } } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); } } else { - throwUnknownToken(token, parser.getTokenLocation()); + parser.skipChildren(); } } return new QueryProfileShardResult(queryProfileResults, rewriteTime, collector); diff --git a/core/src/test/java/org/elasticsearch/search/profile/ProfileResultTests.java b/core/src/test/java/org/elasticsearch/search/profile/ProfileResultTests.java index 77b41b062d..5174267815 100644 --- a/core/src/test/java/org/elasticsearch/search/profile/ProfileResultTests.java +++ b/core/src/test/java/org/elasticsearch/search/profile/ProfileResultTests.java @@ -33,9 +33,11 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; public class ProfileResultTests extends ESTestCase { @@ -62,12 +64,32 @@ public class ProfileResultTests extends ESTestCase { } public void testFromXContent() throws IOException { + doFromXContentTestWithRandomFields(false); + } + + /** + * This test adds random fields and objects to the xContent rendered out to ensure we can parse it + * back to be forward compatible with additions to the xContent + */ + public void testFromXContentWithRandomFields() throws IOException { + doFromXContentTestWithRandomFields(true); + } + + private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws IOException { ProfileResult profileResult = createTestItem(2); XContentType xContentType = randomFrom(XContentType.values()); boolean humanReadable = randomBoolean(); BytesReference originalBytes = toShuffledXContent(profileResult, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); + BytesReference mutated; + if (addRandomFields) { + // "breakdown" just consists of key/value pairs, we shouldn't add anything random there + Predicate<String> excludeFilter = (s) -> s.endsWith(ProfileResult.BREAKDOWN.getPreferredName()); + mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); + } else { + mutated = originalBytes; + } ProfileResult parsed; - try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) { + try (XContentParser parser = createParser(xContentType.xContent(), mutated)) { ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation); parsed = ProfileResult.fromXContent(parser); assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken()); diff --git a/core/src/test/java/org/elasticsearch/search/profile/SearchProfileShardResultsTests.java b/core/src/test/java/org/elasticsearch/search/profile/SearchProfileShardResultsTests.java index 853e7cd13a..7bc9b18860 100644 --- a/core/src/test/java/org/elasticsearch/search/profile/SearchProfileShardResultsTests.java +++ b/core/src/test/java/org/elasticsearch/search/profile/SearchProfileShardResultsTests.java @@ -34,10 +34,12 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName; +import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;; public class SearchProfileShardResultsTests extends ESTestCase { @@ -58,20 +60,43 @@ public class SearchProfileShardResultsTests extends ESTestCase { } public void testFromXContent() throws IOException { + doFromXContentTestWithRandomFields(false); + } + + /** + * This test adds random fields and objects to the xContent rendered out to ensure we can parse it + * back to be forward compatible with additions to the xContent + */ + public void testFromXContentWithRandomFields() throws IOException { + doFromXContentTestWithRandomFields(true); + } + + private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws IOException { SearchProfileShardResults shardResult = createTestItem(); XContentType xContentType = randomFrom(XContentType.values()); boolean humanReadable = randomBoolean(); BytesReference originalBytes = toShuffledXContent(shardResult, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); + BytesReference mutated; + if (addRandomFields) { + // The ProfileResults "breakdown" section just consists of key/value pairs, we shouldn't add anything random there + // also we don't want to insert into the root object here, its just the PROFILE_FIELD itself + Predicate<String> excludeFilter = (s) -> (s.isEmpty() || s.endsWith(ProfileResult.BREAKDOWN.getPreferredName())); + mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); + } else { + mutated = originalBytes; + } SearchProfileShardResults parsed; - try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) { + try (XContentParser parser = createParser(xContentType.xContent(), mutated)) { ensureExpectedToken(parser.nextToken(), XContentParser.Token.START_OBJECT, parser::getTokenLocation); ensureFieldName(parser, parser.nextToken(), SearchProfileShardResults.PROFILE_FIELD); ensureExpectedToken(parser.nextToken(), XContentParser.Token.START_OBJECT, parser::getTokenLocation); parsed = SearchProfileShardResults.fromXContent(parser); + assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken()); assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken()); assertNull(parser.nextToken()); } assertToXContentEquivalent(originalBytes, toXContent(parsed, xContentType, humanReadable), xContentType); + } } diff --git a/core/src/test/java/org/elasticsearch/search/profile/query/CollectorResultTests.java b/core/src/test/java/org/elasticsearch/search/profile/query/CollectorResultTests.java index 8d87f19360..10bf8e2a30 100644 --- a/core/src/test/java/org/elasticsearch/search/profile/query/CollectorResultTests.java +++ b/core/src/test/java/org/elasticsearch/search/profile/query/CollectorResultTests.java @@ -34,6 +34,7 @@ import java.util.List; import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; public class CollectorResultTests extends ESTestCase { @@ -57,18 +58,30 @@ public class CollectorResultTests extends ESTestCase { } public void testFromXContent() throws IOException { + doFromXContentTestWithRandomFields(false); + } + + public void testFromXContentWithRandomFields() throws IOException { + doFromXContentTestWithRandomFields(true); + } + + private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws IOException { CollectorResult collectorResult = createTestItem(1); XContentType xContentType = randomFrom(XContentType.values()); boolean humanReadable = randomBoolean(); BytesReference originalBytes = toShuffledXContent(collectorResult, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); - - CollectorResult parsed; - try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) { + BytesReference mutated; + if (addRandomFields) { + mutated = insertRandomFields(xContentType, originalBytes, null, random()); + } else { + mutated = originalBytes; + } + try (XContentParser parser = createParser(xContentType.xContent(), mutated)) { ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation); - parsed = CollectorResult.fromXContent(parser); + CollectorResult parsed = CollectorResult.fromXContent(parser); assertNull(parser.nextToken()); + assertToXContentEquivalent(originalBytes, toXContent(parsed, xContentType, humanReadable), xContentType); } - assertToXContentEquivalent(originalBytes, toXContent(parsed, xContentType, humanReadable), xContentType); } public void testToXContent() throws IOException { |