[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.

10 files changed, 50 insertions, 13 deletions

diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/Aggregations.java b/core/src/main/java/org/elasticsearch/search/aggregations/Aggregations.java
index 05521e4883..ba51fc419f 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/Aggregations.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/Aggregations.java
@@ -18,10 +18,11 @@
  */
 package org.elasticsearch.search.aggregations;
 
+import org.apache.lucene.util.SetOnce;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.common.xcontent.XContentParserUtils;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -29,10 +30,12 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 
 import static java.util.Collections.unmodifiableMap;
+import static org.elasticsearch.common.xcontent.XContentParserUtils.parseTypedKeysObject;
 
 /**
  * Represents a set of {@link Aggregation}s
@@ -133,7 +136,15 @@ public class Aggregations implements Iterable<Aggregation>, ToXContent {
         XContentParser.Token token;
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
             if (token == XContentParser.Token.START_OBJECT) {
-                aggregations.add(XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class));
+                SetOnce<Aggregation> typedAgg = new SetOnce<>();
+                String currentField = parser.currentName();
+                parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class, typedAgg::set);
+                if (typedAgg.get() != null) {
+                    aggregations.add(typedAgg.get());
+                } else {
+                    throw new ParsingException(parser.getTokenLocation(),
+                            String.format(Locale.ROOT, "Could not parse aggregation keyed as [%s]", currentField));
+                }
             }
         }
         return new Aggregations(aggregations);
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/ParsedMultiBucketAggregation.java b/core/src/main/java/org/elasticsearch/search/aggregations/ParsedMultiBucketAggregation.java
index df80ada8dd..1e601cb30f 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/ParsedMultiBucketAggregation.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/ParsedMultiBucketAggregation.java
@@ -171,7 +171,8 @@ public abstract class ParsedMultiBucketAggregation<B extends ParsedMultiBucketAg
                         bucket.setDocCount(parser.longValue());
                     }
                 } else if (token == XContentParser.Token.START_OBJECT) {
-                    aggregations.add(XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class));
+                    XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class,
+                            aggregations::add);
                 }
             }
             bucket.setAggregations(new Aggregations(aggregations));
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/ParsedSingleBucketAggregation.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/ParsedSingleBucketAggregation.java
index 99d9bfa195..d6f6b2fe3e 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/ParsedSingleBucketAggregation.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/ParsedSingleBucketAggregation.java
@@ -83,7 +83,8 @@ public abstract class ParsedSingleBucketAggregation extends ParsedAggregation im
                 if (CommonFields.META.getPreferredName().equals(currentFieldName)) {
                     aggregation.metadata = parser.map();
                 } else {
-                    aggregations.add(XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class));
+                    XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class,
+                            aggregations::add);
                 }
             }
         }
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/ParsedFilters.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/ParsedFilters.java
index a77577a3cc..e27706d162 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/ParsedFilters.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/ParsedFilters.java
@@ -131,7 +131,8 @@ public class ParsedFilters extends ParsedMultiBucketAggregation<ParsedFilters.Pa
                         bucket.setDocCount(parser.longValue());
                     }
                 } else if (token == XContentParser.Token.START_OBJECT) {
-                    aggregations.add(XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class));
+                    XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class,
+                            aggregations::add);
                 }
             }
             bucket.setAggregations(new Aggregations(aggregations));
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ParsedBinaryRange.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ParsedBinaryRange.java
index 760bd23c09..955724aa66 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ParsedBinaryRange.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ParsedBinaryRange.java
@@ -146,7 +146,8 @@ public class ParsedBinaryRange extends ParsedMultiBucketAggregation<ParsedBinary
                         bucket.to = parser.text();
                     }
                 } else if (token == XContentParser.Token.START_OBJECT) {
-                    aggregations.add(XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class));
+                    XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class,
+                            aggregations::add);
                 }
             }
             bucket.setAggregations(new Aggregations(aggregations));
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ParsedRange.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ParsedRange.java
index 85066296d5..6095348e68 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ParsedRange.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ParsedRange.java
@@ -179,7 +179,8 @@ public class ParsedRange extends ParsedMultiBucketAggregation<ParsedRange.Parsed
                         bucket.toAsString = parser.text();
                     }
                 } else if (token == XContentParser.Token.START_OBJECT) {
-                    aggregations.add(XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class));
+                    XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class,
+                            aggregations::add);
                 }
             }
             bucket.setAggregations(new Aggregations(aggregations));
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/ParsedSignificantTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/ParsedSignificantTerms.java
index 8991ca0993..1b4739c184 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/ParsedSignificantTerms.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/ParsedSignificantTerms.java
@@ -153,6 +153,7 @@ public abstract class ParsedSignificantTerms extends ParsedMultiBucketAggregatio
             return builder;
         }
 
+        @Override
         protected abstract XContentBuilder keyToXContent(XContentBuilder builder) throws IOException;
 
         static <B extends ParsedBucket> B parseSignificantTermsBucketXContent(final XContentParser parser, final B bucket,
@@ -179,7 +180,8 @@ public abstract class ParsedSignificantTerms extends ParsedMultiBucketAggregatio
                         bucket.supersetDf = parser.longValue();
                     }
                 } else if (token == XContentParser.Token.START_OBJECT) {
-                    aggregations.add(XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class));
+                    XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class,
+                            aggregations::add);
                 }
             }
             bucket.setAggregations(new Aggregations(aggregations));
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/ParsedTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/ParsedTerms.java
index 821bb000e3..6aa8bd1624 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/ParsedTerms.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/ParsedTerms.java
@@ -136,7 +136,8 @@ public abstract class ParsedTerms extends ParsedMultiBucketAggregation<ParsedTer
                         bucket.showDocCountError = true;
                     }
                 } else if (token == XContentParser.Token.START_OBJECT) {
-                    aggregations.add(XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class));
+                    XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Aggregation.class,
+                            aggregations::add);
                 }
             }
             bucket.setAggregations(new Aggregations(aggregations));
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/ParsedPercentiles.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/ParsedPercentiles.java
index 48f3dccece..3f56b21dcd 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/ParsedPercentiles.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/ParsedPercentiles.java
@@ -140,6 +140,8 @@ public abstract class ParsedPercentiles extends ParsedAggregation implements Ite
                         }
                     } else if (token == XContentParser.Token.VALUE_NULL) {
                         aggregation.addPercentile(Double.valueOf(parser.currentName()), Double.NaN);
+                    } else {
+                        parser.skipChildren(); // skip potential inner objects and arrays for forward compatibility
                     }
                 }
             } else if (token == XContentParser.Token.START_ARRAY) {
@@ -164,6 +166,8 @@ public abstract class ParsedPercentiles extends ParsedAggregation implements Ite
                             }
                         } else if (token == XContentParser.Token.VALUE_NULL) {
                             value = Double.NaN;
+                        } else {
+                            parser.skipChildren(); // skip potential inner objects and arrays for forward compatibility
                         }
                     }
                     if (key != null) {
diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java
index c2f20f1a79..f55554a445 100644
--- a/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java
+++ b/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java
@@ -19,8 +19,10 @@
 package org.elasticsearch.search.suggest;
 
 import org.apache.lucene.util.CollectionUtil;
+import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.common.CheckedFunction;
 import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Streamable;
@@ -48,6 +50,7 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -177,7 +180,16 @@ public class Suggest implements Iterable<Suggest.Suggestion<? extends Entry<? ex
         ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation);
         List<Suggestion<? extends Entry<? extends Option>>> suggestions = new ArrayList<>();
         while ((parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-            suggestions.add(Suggestion.fromXContent(parser));
+            ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.currentToken(), parser::getTokenLocation);
+            String currentField = parser.currentName();
+            ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.nextToken(), parser::getTokenLocation);
+            Suggestion<? extends Entry<? extends Option>> suggestion = Suggestion.fromXContent(parser);
+            if (suggestion != null) {
+                suggestions.add(suggestion);
+            } else {
+                throw new ParsingException(parser.getTokenLocation(),
+                        String.format(Locale.ROOT, "Could not parse suggestion keyed as [%s]", currentField));
+            }
         }
         return new Suggest(suggestions);
     }
@@ -386,14 +398,16 @@ public class Suggest implements Iterable<Suggest.Suggestion<? extends Entry<? ex
 
         @SuppressWarnings("unchecked")
         public static Suggestion<? extends Entry<? extends Option>> fromXContent(XContentParser parser) throws IOException {
-            ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.currentToken(), parser::getTokenLocation);
-            return XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Suggestion.class);
+            ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser::getTokenLocation);
+            SetOnce<Suggestion> suggestion = new SetOnce<>();
+            XContentParserUtils.parseTypedKeysObject(parser, Aggregation.TYPED_KEYS_DELIMITER, Suggestion.class, suggestion::set);
+            return suggestion.get();
         }
 
         protected static <E extends Suggestion.Entry<?>> void parseEntries(XContentParser parser, Suggestion<E> suggestion,
                                                                            CheckedFunction<XContentParser, E, IOException> entryParser)
                 throws IOException {
-            ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.nextToken(), parser::getTokenLocation);
+            ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser::getTokenLocation);
             while ((parser.nextToken()) != XContentParser.Token.END_ARRAY) {
                 suggestion.addTerm(entryParser.apply(parser));
             }