[Test] Extending parsing checks for SearchResponse (#25148)

This change extends the tests and parsing of SearchResponse to make sure we can
skip additional fields the parser doesn't know for forward compatibility
reasons.

core/src/main/java/org/elasticsearch/action/search/SearchResponse.java

@@ -45,8 +45,6 @@ import java.util.Map;
 
 import static org.elasticsearch.action.search.ShardSearchFailure.readShardSearchFailure;
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
-import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField;
-import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownToken;
 
 
 /**
@@ -245,7 +243,7 @@ public class SearchResponse extends ActionResponse implements StatusToXContentOb
                 } else if (NUM_REDUCE_PHASES.match(currentFieldName)) {
                     numReducePhases = parser.intValue();
                 } else {
-                    throwUnknownField(currentFieldName, parser.getTokenLocation());
+                    parser.skipChildren();
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if (SearchHits.Fields.HITS.equals(currentFieldName)) {
@@ -268,7 +266,7 @@ public class SearchResponse extends ActionResponse implements StatusToXContentOb
                             } else if (RestActions.TOTAL_FIELD.match(currentFieldName)) {
                                 totalShards = parser.intValue();
                             } else {
-                                throwUnknownField(currentFieldName, parser.getTokenLocation());
+                                parser.skipChildren();
                             }
                         } else if (token == XContentParser.Token.START_ARRAY) {
                             if (RestActions.FAILURES_FIELD.match(currentFieldName)) {
@@ -276,14 +274,14 @@ public class SearchResponse extends ActionResponse implements StatusToXContentOb
                                     failures.add(ShardSearchFailure.fromXContent(parser));
                                 }
                             } else {
-                                throwUnknownField(currentFieldName, parser.getTokenLocation());
+                                parser.skipChildren();
                             }
                         } else {
-                            throwUnknownToken(token, parser.getTokenLocation());
+                            parser.skipChildren();
                         }
                     }
                 } else {
-                    throwUnknownField(currentFieldName, parser.getTokenLocation());
+                    parser.skipChildren();
                 }
             }
         }

core/src/test/java/org/elasticsearch/action/search/SearchResponseTests.java

@@ -49,6 +49,7 @@ import java.util.Collections;
 import java.util.List;
 
 import static java.util.Collections.singletonMap;
+import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
 
 public class SearchResponseTests extends ESTestCase {
@@ -78,31 +79,71 @@ public class SearchResponseTests extends ESTestCase {
     }
 
     private SearchResponse createTestItem(ShardSearchFailure... shardSearchFailures) {
-        SearchHits hits = SearchHitsTests.createTestItem();
+        return createTestItem(false, shardSearchFailures);
+    }
+
+    /**
+     * This SearchResponse doesn't include SearchHits, Aggregations, Suggestions, ShardSearchFailures, SearchProfileShardResults
+     * to make it possible to only test properties of the SearchResponse itself
+     */
+    private SearchResponse createMinimalTestItem() {
+        return createTestItem(true);
+    }
+
+    /**
+     * if minimal is set, don't include search hits, aggregations, suggest etc... to make test simpler
+     */
+    private SearchResponse createTestItem(boolean minimal, ShardSearchFailure... shardSearchFailures) {
         boolean timedOut = randomBoolean();
         Boolean terminatedEarly = randomBoolean() ? null : randomBoolean();
         int numReducePhases = randomIntBetween(1, 10);
         long tookInMillis = randomNonNegativeLong();
         int successfulShards = randomInt();
         int totalShards = randomInt();
-
+        InternalSearchResponse internalSearchResponse;
-        InternalAggregations aggregations = aggregationsTests.createTestInstance();
+        if (minimal == false) {
-        Suggest suggest = SuggestTests.createTestItem();
+            SearchHits hits = SearchHitsTests.createTestItem();
-        SearchProfileShardResults profileShardResults = SearchProfileShardResultsTests.createTestItem();
+            InternalAggregations aggregations = aggregationsTests.createTestInstance();
-
+            Suggest suggest = SuggestTests.createTestItem();
-        InternalSearchResponse internalSearchResponse = new InternalSearchResponse(hits, aggregations, suggest, profileShardResults,
+            SearchProfileShardResults profileShardResults = SearchProfileShardResultsTests.createTestItem();
+            internalSearchResponse = new InternalSearchResponse(hits, aggregations, suggest, profileShardResults,
                 timedOut, terminatedEarly, numReducePhases);
+        } else {
+            internalSearchResponse = InternalSearchResponse.empty();
+        }
         return new SearchResponse(internalSearchResponse, null, totalShards, successfulShards, tookInMillis, shardSearchFailures);
     }
 
+    /**
+     * the "_shard/total/failures" section makes it impossible to directly
+     * compare xContent, so we omit it here
+     */
     public void testFromXContent() throws IOException {
-        // the "_shard/total/failures" section makes if impossible to directly compare xContent, so we omit it here
+        doFromXContentTestWithRandomFields(createTestItem(), false);
-        SearchResponse response = createTestItem();
+    }
+
+    /**
+     * 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. We test this with a "minimal" SearchResponse, adding random
+     * fields to SearchHits, Aggregations etc... is tested in their own tests
+     */
+    public void testFromXContentWithRandomFields() throws IOException {
+        doFromXContentTestWithRandomFields(createMinimalTestItem(), true);
+    }
+
+    private void doFromXContentTestWithRandomFields(SearchResponse response, boolean addRandomFields) throws IOException {
         XContentType xcontentType = randomFrom(XContentType.values());
         boolean humanReadable = randomBoolean();
         final ToXContent.Params params = new ToXContent.MapParams(singletonMap(RestSearchAction.TYPED_KEYS_PARAM, "true"));
         BytesReference originalBytes = toShuffledXContent(response, xcontentType, params, humanReadable);
-        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)) {
             SearchResponse parsed = SearchResponse.fromXContent(parser);
             assertToXContentEquivalent(originalBytes, XContentHelper.toXContent(parsed, xcontentType, params, humanReadable), xcontentType);
             assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken());