From 8742db3afe70d8dea9db8a1f9a281ea65f6e77c8 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Tue, 5 Feb 2019 10:08:08 -0600 Subject: [PATCH] Update Rollup Caps to allow unknown fields (#38339) This commit ensures that the parts of rollup caps that can allow unknown fields will allow them. It also modifies the test such that we can use the features we need for disallowing fields in spots where they would not be allowed. Relates #36938 --- .../client/rollup/RollableIndexCaps.java | 2 +- .../client/rollup/RollupJobCaps.java | 26 +++++-------------- .../rollup/GetRollupCapsResponseTests.java | 10 +++++++ .../GetRollupIndexCapsResponseTests.java | 10 +++++++ .../rollup/RollupCapsResponseTestCase.java | 16 +++++++++--- 5 files changed, 40 insertions(+), 24 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollableIndexCaps.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollableIndexCaps.java index cf849e38dd0..8e0bea0996b 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollableIndexCaps.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollableIndexCaps.java @@ -44,7 +44,7 @@ public class RollableIndexCaps implements ToXContentFragment { public static final Function> PARSER = indexName -> { @SuppressWarnings("unchecked") ConstructingObjectParser p - = new ConstructingObjectParser<>(indexName, + = new ConstructingObjectParser<>(indexName, true, a -> new RollableIndexCaps(indexName, (List) a[0])); p.declareObjectArray(ConstructingObjectParser.constructorArg(), RollupJobCaps.PARSER::apply, diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollupJobCaps.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollupJobCaps.java index 7ba1aaa4d7c..15161069f73 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollupJobCaps.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollupJobCaps.java @@ -33,7 +33,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.function.Function; +import java.util.stream.Collectors; /** * Represents the Rollup capabilities for a specific job on a single rollup index @@ -45,15 +45,12 @@ public class RollupJobCaps implements ToXContentObject { private static final ParseField FIELDS = new ParseField("fields"); private static final String NAME = "rollup_job_caps"; - public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, + public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, true, a -> { @SuppressWarnings("unchecked") List> caps = (List>) a[3]; - if (caps.isEmpty()) { - return new RollupJobCaps((String) a[0], (String) a[1], (String) a[2], Collections.emptyMap()); - } - Map mapCaps = new HashMap<>(caps.size()); - caps.forEach(c -> mapCaps.put(c.v1(), c.v2())); + Map mapCaps = + new HashMap<>(caps.stream().collect(Collectors.toMap(Tuple::v1, Tuple::v2))); return new RollupJobCaps((String) a[0], (String) a[1], (String) a[2], mapCaps); }); @@ -140,16 +137,6 @@ public class RollupJobCaps implements ToXContentObject { private static final String NAME = "rollup_field_caps"; private final List> aggs; - public static final Function> PARSER = fieldName -> { - @SuppressWarnings("unchecked") - ConstructingObjectParser parser - = new ConstructingObjectParser<>(NAME, a -> new RollupFieldCaps((List>) a[0])); - - parser.declareObjectArray(ConstructingObjectParser.constructorArg(), - (p, c) -> p.map(), new ParseField(fieldName)); - return parser; - }; - RollupFieldCaps(final List> aggs) { this.aggs = Collections.unmodifiableList(Objects.requireNonNull(aggs)); } @@ -170,13 +157,12 @@ public class RollupJobCaps implements ToXContentObject { List> aggs = new ArrayList<>(); if (parser.nextToken().equals(XContentParser.Token.START_ARRAY)) { while (parser.nextToken().equals(XContentParser.Token.START_OBJECT)) { - aggs.add(Collections.unmodifiableMap(parser.map())); + aggs.add(parser.map()); } } - return new RollupFieldCaps(Collections.unmodifiableList(aggs)); + return new RollupFieldCaps(aggs); } - @Override public boolean equals(Object other) { if (this == other) { diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupCapsResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupCapsResponseTests.java index a728b65cf64..a9c3a59faf5 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupCapsResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupCapsResponseTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.Map; +import java.util.function.Predicate; public class GetRollupCapsResponseTests extends RollupCapsResponseTestCase { @@ -40,6 +41,15 @@ public class GetRollupCapsResponseTests extends RollupCapsResponseTestCase randomFieldsExcludeFilter() { + return (field) -> + // base cannot have extra things in it + "".equals(field) + // the field list expects to be a nested object of a certain type + || field.contains("fields"); + } + @Override protected GetRollupCapsResponse fromXContent(XContentParser parser) throws IOException { return GetRollupCapsResponse.fromXContent(parser); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupIndexCapsResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupIndexCapsResponseTests.java index afd0e54f92b..20e29aef0df 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupIndexCapsResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupIndexCapsResponseTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.Map; +import java.util.function.Predicate; public class GetRollupIndexCapsResponseTests extends RollupCapsResponseTestCase { @@ -40,6 +41,15 @@ public class GetRollupIndexCapsResponseTests extends RollupCapsResponseTestCase< builder.endObject(); } + @Override + protected Predicate randomFieldsExcludeFilter() { + return (field) -> + // base cannot have extra things in it + "".equals(field) + // the field list expects to be a nested object of a certain type + || field.contains("fields"); + } + @Override protected GetRollupIndexCapsResponse fromXContent(XContentParser parser) throws IOException { return GetRollupIndexCapsResponse.fromXContent(parser); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/RollupCapsResponseTestCase.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/RollupCapsResponseTestCase.java index 6d1c0359d17..cdc4280dbff 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/RollupCapsResponseTestCase.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/RollupCapsResponseTestCase.java @@ -25,6 +25,7 @@ import org.elasticsearch.client.rollup.job.config.MetricConfig; import org.elasticsearch.client.rollup.job.config.RollupJobConfig; import org.elasticsearch.client.rollup.job.config.RollupJobConfigTests; import org.elasticsearch.client.rollup.job.config.TermsGroupConfig; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; @@ -40,6 +41,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import java.util.stream.Collectors; import static java.util.Collections.singletonMap; @@ -55,15 +57,23 @@ abstract class RollupCapsResponseTestCase extends ESTestCase { protected abstract T fromXContent(XContentParser parser) throws IOException; + protected Predicate randomFieldsExcludeFilter() { + return field -> false; + } + + protected String[] shuffleFieldsExceptions() { + return Strings.EMPTY_ARRAY; + } + public void testFromXContent() throws IOException { xContentTester( this::createParser, this::createTestInstance, this::toXContent, this::fromXContent) - .supportsUnknownFields(false) - .randomFieldsExcludeFilter(field -> - field.endsWith("job_id")) + .supportsUnknownFields(true) + .randomFieldsExcludeFilter(randomFieldsExcludeFilter()) + .shuffleFieldsExceptions(shuffleFieldsExceptions()) .test(); }