[Tests] Check that parsing aggregations works in a forward compatible way (#25219)

This change adds tests for the aggregation parsing that try to simulate that we
can parse existing aggregations in a forward compatible way in the future,
ignoring potential newly added fields or substructures to the xContent response.

3 files changed, 73 insertions, 37 deletions

diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalAggregationTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/InternalAggregationTestCase.java
index 90cec479f1..d7b9b186c9 100644
--- a/test/framework/src/main/java/org/elasticsearch/test/InternalAggregationTestCase.java
+++ b/test/framework/src/main/java/org/elasticsearch/test/InternalAggregationTestCase.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.test;
 
+import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
@@ -130,12 +131,14 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
+import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
 
 public abstract class InternalAggregationTestCase<T extends InternalAggregation> extends AbstractWireSerializingTestCase<T> {
@@ -297,7 +300,13 @@ public abstract class InternalAggregationTestCase<T extends InternalAggregation>
 
     public final void testFromXContent() throws IOException {
         final T aggregation = createTestInstance();
-        final Aggregation parsedAggregation = parseAndAssert(aggregation, randomBoolean());
+        final Aggregation parsedAggregation = parseAndAssert(aggregation, randomBoolean(), false);
+        assertFromXContent(aggregation, (ParsedAggregation) parsedAggregation);
+    }
+
+    public final void testFromXContentWithRandomFields() throws IOException {
+        final T aggregation = createTestInstance();
+        final Aggregation parsedAggregation = parseAndAssert(aggregation, randomBoolean(), true);
         assertFromXContent(aggregation, (ParsedAggregation) parsedAggregation);
     }
 
@@ -305,7 +314,7 @@ public abstract class InternalAggregationTestCase<T extends InternalAggregation>
 
     @SuppressWarnings("unchecked")
     protected <P extends ParsedAggregation> P parseAndAssert(final InternalAggregation aggregation,
-                                                             final boolean shuffled) throws IOException {
+                                                             final boolean shuffled, final boolean addRandomFields) throws IOException {
 
         final ToXContent.Params params = new ToXContent.MapParams(singletonMap(RestSearchAction.TYPED_KEYS_PARAM, "true"));
         final XContentType xContentType = randomFrom(XContentType.values());
@@ -317,29 +326,57 @@ public abstract class InternalAggregationTestCase<T extends InternalAggregation>
         } else {
             originalBytes = toXContent(aggregation, xContentType, params, humanReadable);
         }
+        BytesReference mutated;
+        if (addRandomFields) {
+            /*
+             * - we don't add to the root object because it should only contain
+             * the named aggregation to test - we don't want to insert into the
+             * "meta" object, because we pass on everything we find there
+             *
+             * - we don't want to directly insert anything random into "buckets"
+             * objects, they are used with "keyed" aggregations and contain
+             * named bucket objects. Any new named object on this level should
+             * also be a bucket and be parsed as such.
+             */
+            Predicate<String> basicExcludes = path -> path.isEmpty() || path.endsWith(Aggregation.CommonFields.META.getPreferredName())
+                    || path.endsWith(Aggregation.CommonFields.BUCKETS.getPreferredName());
+            Predicate<String> excludes = basicExcludes.or(excludePathsFromXContentInsertion());
+            mutated = insertRandomFields(xContentType, originalBytes, excludes, random());
+        } else {
+            mutated = originalBytes;
+        }
 
-        Aggregation parsedAggregation;
-        try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
+        SetOnce<Aggregation> parsedAggregation = new SetOnce<>();
+        try (XContentParser parser = createParser(xContentType.xContent(), mutated)) {
             assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
             assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken());
-
-            parsedAggregation = XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class);
+            assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
+            XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class, parsedAggregation::set);
 
             assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken());
             assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());
             assertNull(parser.nextToken());
 
-            assertEquals(aggregation.getName(), parsedAggregation.getName());
-            assertEquals(aggregation.getMetaData(), parsedAggregation.getMetaData());
+            Aggregation agg = parsedAggregation.get();
+            assertEquals(aggregation.getName(), agg.getName());
+            assertEquals(aggregation.getMetaData(), agg.getMetaData());
+
+            assertTrue(agg instanceof ParsedAggregation);
+            assertEquals(aggregation.getType(), agg.getType());
+
+            BytesReference parsedBytes = toXContent(agg, xContentType, params, humanReadable);
+            assertToXContentEquivalent(originalBytes, parsedBytes, xContentType);
 
-            assertTrue(parsedAggregation instanceof ParsedAggregation);
-            assertEquals(aggregation.getType(), parsedAggregation.getType());
+            return (P) agg;
         }
 
-        BytesReference parsedBytes = toXContent(parsedAggregation, xContentType, params, humanReadable);
-        assertToXContentEquivalent(originalBytes, parsedBytes, xContentType);
+    }
 
-        return (P) parsedAggregation;
+    /**
+     * Overwrite this in your test if other than the basic xContent paths should be excluded during insertion of random fields
+     */
+    protected Predicate<String> excludePathsFromXContentInsertion() {
+        return path -> false;
     }
 
     /**
diff --git a/test/framework/src/main/java/org/elasticsearch/test/XContentTestUtils.java b/test/framework/src/main/java/org/elasticsearch/test/XContentTestUtils.java
index 16953b45d1..c8a0ce8fc2 100644
--- a/test/framework/src/main/java/org/elasticsearch/test/XContentTestUtils.java
+++ b/test/framework/src/main/java/org/elasticsearch/test/XContentTestUtils.java
@@ -32,13 +32,13 @@ import org.elasticsearch.test.rest.yaml.ObjectPath;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import java.util.Stack;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 import static com.carrotsearch.randomizedtesting.generators.RandomStrings.randomAsciiOfLength;
 import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS;
@@ -195,22 +195,20 @@ public final class XContentTestUtils {
             }
         }
 
-        try (XContentParser parser = createParser(NamedXContentRegistry.EMPTY, xContent, contentType)) {
-            Supplier<Object> value = () -> {
+        Supplier<Object> value = () -> {
+            List<Object> randomValues = RandomObjects.randomStoredFieldValues(random, contentType).v1();
+            if (random.nextBoolean()) {
+                return randomValues.get(0);
+            } else {
                 if (random.nextBoolean()) {
-                    return RandomObjects.randomStoredFieldValues(random, contentType);
+                    return randomValues.stream().collect(Collectors.toMap(obj -> randomAsciiOfLength(random, 10), obj -> obj));
                 } else {
-                    if (random.nextBoolean()) {
-                        return Collections.singletonMap(randomAsciiOfLength(random, 10), randomAsciiOfLength(random, 10));
-                    } else {
-                        return Collections.singletonList(randomAsciiOfLength(random, 10));
-                    }
+                    return randomValues;
                 }
-            };
-            return XContentTestUtils
-                    .insertIntoXContent(contentType.xContent(), xContent, insertPaths, () -> randomAsciiOfLength(random, 10), value)
-                    .bytes();
-        }
+            }
+        };
+        return XContentTestUtils
+                .insertIntoXContent(contentType.xContent(), xContent, insertPaths, () -> randomAsciiOfLength(random, 10), value).bytes();
     }
 
     /**
@@ -251,7 +249,8 @@ public final class XContentTestUtils {
         List<String> validPaths = new ArrayList<>();
         // parser.currentName() can be null for root object and unnamed objects in arrays
         if (parser.currentName() != null) {
-            currentPath.push(parser.currentName());
+            // dots in randomized field names need to be escaped, we use that character as the path separator
+            currentPath.push(parser.currentName().replaceAll("\\.", "\\\\."));
         }
         if (parser.currentToken() == XContentParser.Token.START_OBJECT) {
             validPaths.add(String.join(".", currentPath.toArray(new String[currentPath.size()])));
diff --git a/test/framework/src/test/java/org/elasticsearch/test/XContentTestUtilsTests.java b/test/framework/src/test/java/org/elasticsearch/test/XContentTestUtilsTests.java
index 3897064550..f3b44f2510 100644
--- a/test/framework/src/test/java/org/elasticsearch/test/XContentTestUtilsTests.java
+++ b/test/framework/src/test/java/org/elasticsearch/test/XContentTestUtilsTests.java
@@ -61,7 +61,7 @@ public class XContentTestUtilsTests extends ESTestCase {
             builder.startObject("inner1");
             {
                 builder.field("inner1field1", "value");
-                builder.startObject("inner2");
+                builder.startObject("inn.er2");
                 {
                     builder.field("inner2field1", "value");
                 }
@@ -79,7 +79,7 @@ public class XContentTestUtilsTests extends ESTestCase {
             assertThat(insertPaths, hasItem(equalTo("list1.2")));
             assertThat(insertPaths, hasItem(equalTo("list1.4")));
             assertThat(insertPaths, hasItem(equalTo("inner1")));
-            assertThat(insertPaths, hasItem(equalTo("inner1.inner2")));
+            assertThat(insertPaths, hasItem(equalTo("inner1.inn\\.er2")));
         }
     }
 
@@ -89,19 +89,19 @@ public class XContentTestUtilsTests extends ESTestCase {
         builder.startObject();
         builder.endObject();
         builder = XContentTestUtils.insertIntoXContent(XContentType.JSON.xContent(), builder.bytes(), Collections.singletonList(""),
-                () -> "inner1", () -> new HashMap<>());
+                () -> "inn.er1", () -> new HashMap<>());
         builder = XContentTestUtils.insertIntoXContent(XContentType.JSON.xContent(), builder.bytes(), Collections.singletonList(""),
                 () -> "field1", () -> "value1");
-        builder = XContentTestUtils.insertIntoXContent(XContentType.JSON.xContent(), builder.bytes(), Collections.singletonList("inner1"),
-                () -> "inner2", () -> new HashMap<>());
-        builder = XContentTestUtils.insertIntoXContent(XContentType.JSON.xContent(), builder.bytes(), Collections.singletonList("inner1"),
-                () -> "field2", () -> "value2");
+        builder = XContentTestUtils.insertIntoXContent(XContentType.JSON.xContent(), builder.bytes(),
+                Collections.singletonList("inn\\.er1"), () -> "inner2", () -> new HashMap<>());
+        builder = XContentTestUtils.insertIntoXContent(XContentType.JSON.xContent(), builder.bytes(),
+                Collections.singletonList("inn\\.er1"), () -> "field2", () -> "value2");
         try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, builder.bytes(), builder.contentType())) {
             Map<String, Object> map = parser.map();
             assertEquals(2, map.size());
             assertEquals("value1", map.get("field1"));
-            assertThat(map.get("inner1"), instanceOf(Map.class));
-            Map<String, Object> innerMap = (Map<String, Object>) map.get("inner1");
+            assertThat(map.get("inn.er1"), instanceOf(Map.class));
+            Map<String, Object> innerMap = (Map<String, Object>) map.get("inn.er1");
             assertEquals(2, innerMap.size());
             assertEquals("value2", innerMap.get("field2"));
             assertThat(innerMap.get("inner2"), instanceOf(Map.class));