[Test] Adding test for parsing SearchShardFailure leniently (#25144)

This change extends the tests and parsing of SearchShardFailure to make sure we can skip fields the parser doesn't know for forward compatibility reasons.
elastic · Jun 9, 2017 · eca4f24 · eca4f24
1 parent 34c3d1d
commit eca4f24
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 9 deletions.
diff --git a/core/src/main/java/org/elasticsearch/action/search/ShardSearchFailure.java b/core/src/main/java/org/elasticsearch/action/search/ShardSearchFailure.java
@@ -38,8 +38,6 @@
 import java.io.IOException;
 
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
-import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField;
-import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownToken;
 
 /**
  * Represents a failure to search on a specific shard.
@@ -200,16 +198,16 @@ public static ShardSearchFailure fromXContent(XContentParser parser) throws IOEx
                 } else if (NODE_FIELD.equals(currentFieldName)) {
                     nodeId  = parser.text();
                 } else {
-                    throwUnknownField(currentFieldName, parser.getTokenLocation());
+                    parser.skipChildren();
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if (REASON_FIELD.equals(currentFieldName)) {
                     exception = ElasticsearchException.fromXContent(parser);
                 } else {
-                    throwUnknownField(currentFieldName, parser.getTokenLocation());
+                    parser.skipChildren();
                 }
             } else {
-                throwUnknownToken(token, parser.getTokenLocation());
+                parser.skipChildren();
             }
         }
         return new ShardSearchFailure(exception,

diff --git a/core/src/test/java/org/elasticsearch/action/search/ShardSearchFailureTests.java b/core/src/test/java/org/elasticsearch/action/search/ShardSearchFailureTests.java
@@ -33,6 +33,7 @@
 import java.io.IOException;
 
 import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
+import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
 
 public class ShardSearchFailureTests extends ESTestCase {
 
@@ -48,13 +49,31 @@ public static ShardSearchFailure createTestItem() {
     }
 
     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 {
         ShardSearchFailure response = createTestItem();
         XContentType xContentType = randomFrom(XContentType.values());
         boolean humanReadable = randomBoolean();
         BytesReference originalBytes = toShuffledXContent(response, xContentType, ToXContent.EMPTY_PARAMS, humanReadable);
-
+        BytesReference mutated;
+        if (addRandomFields) {
+            mutated = insertRandomFields(xContentType, originalBytes, null, random());
+        } else {
+            mutated = originalBytes;
+        }
         ShardSearchFailure parsed;
-        try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
+        try (XContentParser parser = createParser(xContentType.xContent(), mutated)) {
             assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
             parsed = ShardSearchFailure.fromXContent(parser);
             assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken());
@@ -64,8 +83,11 @@ public void testFromXContent() throws IOException {
         assertEquals(response.shard().getNodeId(), parsed.shard().getNodeId());
         assertEquals(response.shardId(), parsed.shardId());
 
-        // we cannot compare the cause, because it will be wrapped in an outer ElasticSearchException
-        // best effort: try to check that the original message appears somewhere in the rendered xContent
+        /**
+         * we cannot compare the cause, because it will be wrapped in an outer
+         * ElasticSearchException best effort: try to check that the original
+         * message appears somewhere in the rendered xContent
+         */
         String originalMsg = response.getCause().getMessage();
         assertEquals(parsed.getCause().getMessage(), "Elasticsearch exception [type=parsing_exception, reason=" + originalMsg + "]");
         String nestedMsg = response.getCause().getCause().getMessage();