From 39da806406473d36e01345af6a22ee68ff20b8f4 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Fri, 1 Feb 2019 16:02:56 -0600 Subject: [PATCH 1/2] Update Rollup Caps to allow unknown fields 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/GetRollupCapsResponse.java | 4 +-- .../client/rollup/RollableIndexCaps.java | 2 +- .../client/rollup/RollupJobCaps.java | 34 ++++++------------- .../rollup/GetRollupCapsResponseTests.java | 10 ++++++ .../GetRollupIndexCapsResponseTests.java | 10 ++++++ .../rollup/RollupCapsResponseTestCase.java | 16 +++++++-- 6 files changed, 46 insertions(+), 30 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java index f4c516d015d78..9899251f92f45 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java @@ -22,9 +22,9 @@ import java.io.IOException; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.TreeMap; public class GetRollupCapsResponse { @@ -39,7 +39,7 @@ public Map getJobs() { } public static GetRollupCapsResponse fromXContent(final XContentParser parser) throws IOException { - Map jobs = new HashMap<>(); + Map jobs = new TreeMap<>(); XContentParser.Token token = parser.nextToken(); if (token.equals(XContentParser.Token.START_OBJECT)) { while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { 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 cf849e38dd0b4..8e0bea0996bbd 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 7ba1aaa4d7c2b..6d80b193af0eb 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 @@ -19,7 +19,6 @@ package org.elasticsearch.client.rollup; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.ToXContentObject; @@ -27,13 +26,14 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.function.Function; +import java.util.TreeMap; +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())); + List> caps = (List>) a[3]; + Map mapCaps = + new TreeMap<>(caps.stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); return new RollupJobCaps((String) a[0], (String) a[1], (String) a[2], mapCaps); }); @@ -62,7 +59,7 @@ public class RollupJobCaps implements ToXContentObject { PARSER.declareString(ConstructingObjectParser.constructorArg(), ROLLUP_INDEX); PARSER.declareString(ConstructingObjectParser.constructorArg(), INDEX_PATTERN); PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(), - (p, c, name) -> new Tuple<>(name, RollupFieldCaps.fromXContent(p)), FIELDS); + (p, c, name) -> new AbstractMap.SimpleEntry<>(name, RollupFieldCaps.fromXContent(p)), FIELDS); } private final String jobID; @@ -140,16 +137,6 @@ public static class RollupFieldCaps implements ToXContentFragment { 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 static RollupFieldCaps fromXContent(XContentParser parser) throws IOExcep 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 a728b65cf64ce..a9c3a59faf5ae 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 java.io.IOException; import java.util.Map; +import java.util.function.Predicate; public class GetRollupCapsResponseTests extends RollupCapsResponseTestCase { @@ -40,6 +41,15 @@ protected void toXContent(GetRollupCapsResponse response, XContentBuilder builde 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 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 afd0e54f92b1f..20e29aef0df64 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 java.io.IOException; import java.util.Map; +import java.util.function.Predicate; public class GetRollupIndexCapsResponseTests extends RollupCapsResponseTestCase { @@ -40,6 +41,15 @@ protected void toXContent(GetRollupIndexCapsResponse response, XContentBuilder b 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 6d1c0359d172d..cdc4280dbff91 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.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.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(); } From 32f8d2ebee661cbb0ba902abbeb0953dde1247e3 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Mon, 4 Feb 2019 14:46:04 -0600 Subject: [PATCH 2/2] Fixup for review --- .../client/rollup/GetRollupCapsResponse.java | 4 ++-- .../org/elasticsearch/client/rollup/RollupJobCaps.java | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java index 9899251f92f45..f4c516d015d78 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java @@ -22,9 +22,9 @@ import java.io.IOException; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.TreeMap; public class GetRollupCapsResponse { @@ -39,7 +39,7 @@ public Map getJobs() { } public static GetRollupCapsResponse fromXContent(final XContentParser parser) throws IOException { - Map jobs = new TreeMap<>(); + Map jobs = new HashMap<>(); XContentParser.Token token = parser.nextToken(); if (token.equals(XContentParser.Token.START_OBJECT)) { while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { 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 6d80b193af0eb..15161069f7338 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 @@ -19,6 +19,7 @@ package org.elasticsearch.client.rollup; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.ToXContentObject; @@ -26,13 +27,12 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; -import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.TreeMap; import java.util.stream.Collectors; /** @@ -48,9 +48,9 @@ public class RollupJobCaps implements ToXContentObject { public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, true, a -> { @SuppressWarnings("unchecked") - List> caps = (List>) a[3]; + List> caps = (List>) a[3]; Map mapCaps = - new TreeMap<>(caps.stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); + new HashMap<>(caps.stream().collect(Collectors.toMap(Tuple::v1, Tuple::v2))); return new RollupJobCaps((String) a[0], (String) a[1], (String) a[2], mapCaps); }); @@ -59,7 +59,7 @@ public class RollupJobCaps implements ToXContentObject { PARSER.declareString(ConstructingObjectParser.constructorArg(), ROLLUP_INDEX); PARSER.declareString(ConstructingObjectParser.constructorArg(), INDEX_PATTERN); PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(), - (p, c, name) -> new AbstractMap.SimpleEntry<>(name, RollupFieldCaps.fromXContent(p)), FIELDS); + (p, c, name) -> new Tuple<>(name, RollupFieldCaps.fromXContent(p)), FIELDS); } private final String jobID;