From 4f4e07b444a8b6acc6c64067e7498ce236e5096a Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 31 Jul 2018 19:08:51 +0200 Subject: [PATCH] [Rollup] Remove builders from TermsGroupConfig Remove --- .../xpack/core/rollup/job/GroupConfig.java | 13 ++- .../core/rollup/job/TermsGroupConfig.java | 83 ++++++++----------- .../xpack/core/rollup/ConfigTestHelpers.java | 29 +++++-- .../job/TermsGroupConfigSerializingTests.java | 44 +++------- .../xpack/rollup/job/RollupIndexer.java | 1 - .../rollup/RollupJobIdentifierUtilTests.java | 5 +- .../rollup/action/SearchActionTests.java | 7 +- .../xpack/rollup/config/ConfigTests.java | 15 ++-- .../xpack/rollup/job/IndexerUtilsTests.java | 2 +- 9 files changed, 88 insertions(+), 111 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java index dcb393b088618..f6977394f4af2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java @@ -23,6 +23,8 @@ import java.util.Objects; import java.util.Set; +import static java.util.Arrays.asList; + /** * The configuration object for the groups section in the rollup config. * Basically just a wrapper for histo/date histo/terms objects @@ -50,7 +52,7 @@ public class GroupConfig implements Writeable, ToXContentObject { static { PARSER.declareObject(GroupConfig.Builder::setDateHisto, (p,c) -> DateHistoGroupConfig.PARSER.apply(p,c).build(), DATE_HISTO); PARSER.declareObject(GroupConfig.Builder::setHisto, (p,c) -> HistoGroupConfig.PARSER.apply(p,c).build(), HISTO); - PARSER.declareObject(GroupConfig.Builder::setTerms, (p,c) -> TermsGroupConfig.PARSER.apply(p,c).build(), TERMS); + PARSER.declareObject(GroupConfig.Builder::setTerms, (p,c) -> TermsGroupConfig.fromXContent(p), TERMS); } private GroupConfig(DateHistoGroupConfig dateHisto, @Nullable HistoGroupConfig histo, @Nullable TermsGroupConfig terms) { @@ -84,7 +86,7 @@ public Set getAllFields() { fields.addAll(histo.getAllFields()); } if (terms != null) { - fields.addAll(terms.getAllFields()); + fields.addAll(asList(terms.getFields())); } return fields; } @@ -100,7 +102,6 @@ public void validateMappings(Map> fieldCa } } - @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); @@ -113,9 +114,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); } if (terms != null) { - builder.startObject(TERMS.getPreferredName()); - terms.toXContent(builder, params); - builder.endObject(); + builder.field(TERMS.getPreferredName(), terms); } builder.endObject(); return builder; @@ -194,4 +193,4 @@ public GroupConfig build() { return new GroupConfig(dateHisto, histo, terms); } } -} \ No newline at end of file +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfig.java index da73020f0087f..a1b0b3118ec5d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfig.java @@ -12,9 +12,10 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ObjectParser; -import org.elasticsearch.common.xcontent.ToXContentFragment; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.TextFieldMapper; import org.elasticsearch.search.aggregations.bucket.composite.CompositeValuesSourceBuilder; @@ -24,13 +25,13 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.stream.Collectors; +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; + /** * The configuration object for the histograms in the rollup config * @@ -42,20 +43,29 @@ * ] * } */ -public class TermsGroupConfig implements Writeable, ToXContentFragment { - private static final String NAME = "term_group_config"; - public static final ObjectParser PARSER = new ObjectParser<>(NAME, TermsGroupConfig.Builder::new); +public class TermsGroupConfig implements Writeable, ToXContentObject { + + private static final String NAME = "terms"; + private static final String FIELDS = "fields"; - private static final ParseField FIELDS = new ParseField("fields"); private static final List FLOAT_TYPES = Arrays.asList("half_float", "float", "double", "scaled_float"); private static final List NATURAL_TYPES = Arrays.asList("byte", "short", "integer", "long"); - private final String[] fields; + private static final ConstructingObjectParser PARSER; static { - PARSER.declareStringArray(TermsGroupConfig.Builder::setFields, FIELDS); + PARSER = new ConstructingObjectParser<>(NAME, args -> { + @SuppressWarnings("unchecked") List fields = (List) args[0]; + return new TermsGroupConfig(fields != null ? fields.toArray(new String[fields.size()]) : null); + }); + PARSER.declareStringArray(constructorArg(), new ParseField(FIELDS)); } - private TermsGroupConfig(String[] fields) { + private final String[] fields; + + public TermsGroupConfig(final String... fields) { + if (fields == null || fields.length == 0) { + throw new IllegalArgumentException("Fields must have at least one value"); + } this.fields = fields; } @@ -63,6 +73,9 @@ private TermsGroupConfig(String[] fields) { fields = in.readStringArray(); } + /** + * @return the names of the fields. Never {@code null}. + */ public String[] getFields() { return fields; } @@ -72,10 +85,6 @@ public String[] getFields() { * set of date histograms. Used by the rollup indexer to iterate over historical data */ public List> toBuilders() { - if (fields.length == 0) { - return Collections.emptyList(); - } - return Arrays.stream(fields).map(f -> { TermsValuesSourceBuilder vsBuilder = new TermsValuesSourceBuilder(RollupField.formatIndexerAggName(f, TermsAggregationBuilder.NAME)); @@ -94,14 +103,6 @@ public Map toAggCap() { return map; } - public Map getMetadata() { - return Collections.emptyMap(); - } - - public Set getAllFields() { - return Arrays.stream(fields).collect(Collectors.toSet()); - } - public void validateMappings(Map> fieldCapsResponse, ActionRequestValidationException validationException) { @@ -138,8 +139,11 @@ public void validateMappings(Map> fieldCa @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(FIELDS.getPreferredName(), fields); - return builder; + builder.startObject(); + { + builder.field(FIELDS, fields); + } + return builder.endObject(); } @Override @@ -148,18 +152,15 @@ public void writeTo(StreamOutput out) throws IOException { } @Override - public boolean equals(Object other) { + public boolean equals(final Object other) { if (this == other) { return true; } - if (other == null || getClass() != other.getClass()) { return false; } - - TermsGroupConfig that = (TermsGroupConfig) other; - - return Arrays.equals(this.fields, that.fields); + final TermsGroupConfig that = (TermsGroupConfig) other; + return Arrays.equals(fields, that.fields); } @Override @@ -172,23 +173,7 @@ public String toString() { return Strings.toString(this, true, true); } - public static class Builder { - private List fields; - - public List getFields() { - return fields; - } - - public TermsGroupConfig.Builder setFields(List fields) { - this.fields = fields; - return this; - } - - public TermsGroupConfig build() { - if (fields == null || fields.isEmpty()) { - throw new IllegalArgumentException("Parameter [" + FIELDS + "] must have at least one value."); - } - return new TermsGroupConfig(fields.toArray(new String[0])); - } + public static TermsGroupConfig fromXContent(final XContentParser parser) throws IOException { + return PARSER.parse(parser, null); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java index 3e4e4a84d2f8e..b2e2730760f9c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java @@ -17,9 +17,13 @@ import java.util.ArrayList; import java.util.List; +import java.util.Random; import java.util.stream.Collectors; import java.util.stream.IntStream; +import static com.carrotsearch.randomizedtesting.generators.RandomNumbers.randomIntBetween; +import static com.carrotsearch.randomizedtesting.generators.RandomStrings.randomAsciiAlphanumOfLengthBetween; + public class ConfigTestHelpers { public static RollupJobConfig.Builder getRollupJob(String jobId) { @@ -49,7 +53,7 @@ public static GroupConfig.Builder getGroupConfig() { groupBuilder.setHisto(getHisto().build()); } if (ESTestCase.randomBoolean()) { - groupBuilder.setTerms(getTerms().build()); + groupBuilder.setTerms(randomTermsGroupConfig(ESTestCase.random())); } return groupBuilder; } @@ -105,12 +109,6 @@ public static HistoGroupConfig.Builder getHisto() { return histoBuilder; } - public static TermsGroupConfig.Builder getTerms() { - TermsGroupConfig.Builder builder = new TermsGroupConfig.Builder(); - builder.setFields(getFields()); - return builder; - } - public static List getFields() { return IntStream.range(0, ESTestCase.randomIntBetween(1, 10)) .mapToObj(n -> ESTestCase.randomAlphaOfLengthBetween(5, 10)) @@ -126,4 +124,21 @@ public static String getCronString() { " ?" + //day of week " " + (ESTestCase.randomBoolean() ? "*" : String.valueOf(ESTestCase.randomIntBetween(1970, 2199))); //year } + + public static TermsGroupConfig randomTermsGroupConfig(final Random random) { + return new TermsGroupConfig(randomFields(random)); + } + + private static String[] randomFields(final Random random) { + final int numFields = randomIntBetween(random, 1, 10); + final String[] fields = new String[numFields]; + for (int i = 0; i < numFields; i++) { + fields[i] = randomField(random); + } + return fields; + } + + private static String randomField(final Random random) { + return randomAsciiAlphanumOfLengthBetween(random, 5, 10); + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfigSerializingTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfigSerializingTests.java index f48f678fac85a..ccdd616df7b51 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfigSerializingTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfigSerializingTests.java @@ -11,27 +11,23 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.aggregations.bucket.composite.CompositeValuesSourceBuilder; import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers; import java.io.IOException; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomTermsGroupConfig; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class TermsGroupConfigSerializingTests extends AbstractSerializingTestCase { - private static final List FLOAT_TYPES = Arrays.asList("half_float", "float", "double", "scaled_float"); - private static final List NATURAL_TYPES = Arrays.asList("byte", "short", "integer", "long"); - @Override protected TermsGroupConfig doParseInstance(XContentParser parser) throws IOException { - return TermsGroupConfig.PARSER.apply(parser, null).build(); + return TermsGroupConfig.fromXContent(parser); } @Override @@ -41,23 +37,20 @@ protected Writeable.Reader instanceReader() { @Override protected TermsGroupConfig createTestInstance() { - return ConfigTestHelpers.getTerms().build(); + return randomTermsGroupConfig(random()); } - public void testValidateNoMapping() throws IOException { + public void testValidateNoMapping() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); - TermsGroupConfig config = new TermsGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .build(); + TermsGroupConfig config = new TermsGroupConfig("my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] or [keyword/text] field with name " + "[my_field] in any of the indices matching the index pattern.")); } - public void testValidateNomatchingField() throws IOException { - + public void testValidateNomatchingField() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); @@ -65,16 +58,13 @@ public void testValidateNomatchingField() throws IOException { FieldCapabilities fieldCaps = mock(FieldCapabilities.class); responseMap.put("some_other_field", Collections.singletonMap("keyword", fieldCaps)); - TermsGroupConfig config = new TermsGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .build(); + TermsGroupConfig config = new TermsGroupConfig("my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] or [keyword/text] field with name " + "[my_field] in any of the indices matching the index pattern.")); } - public void testValidateFieldWrongType() throws IOException { - + public void testValidateFieldWrongType() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); @@ -82,17 +72,13 @@ public void testValidateFieldWrongType() throws IOException { FieldCapabilities fieldCaps = mock(FieldCapabilities.class); responseMap.put("my_field", Collections.singletonMap("geo_point", fieldCaps)); - TermsGroupConfig config = new TermsGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .build(); + TermsGroupConfig config = new TermsGroupConfig("my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("The field referenced by a terms group must be a [numeric] or " + "[keyword/text] type, but found [geo_point] for field [my_field]")); } - public void testValidateFieldMatchingNotAggregatable() throws IOException { - - + public void testValidateFieldMatchingNotAggregatable() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); @@ -101,14 +87,12 @@ public void testValidateFieldMatchingNotAggregatable() throws IOException { when(fieldCaps.isAggregatable()).thenReturn(false); responseMap.put("my_field", Collections.singletonMap(getRandomType(), fieldCaps)); - TermsGroupConfig config = new TermsGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .build(); + TermsGroupConfig config = new TermsGroupConfig("my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable across all indices, but is not.")); } - public void testValidateMatchingField() throws IOException { + public void testValidateMatchingField() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); String type = getRandomType(); @@ -118,9 +102,7 @@ public void testValidateMatchingField() throws IOException { when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap(type, fieldCaps)); - TermsGroupConfig config = new TermsGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .build(); + TermsGroupConfig config = new TermsGroupConfig("my_field"); config.validateMappings(responseMap, e); if (e.validationErrors().size() != 0) { fail(e.getMessage()); diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java index c7d29451ab3be..8b12ae6348012 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java @@ -391,7 +391,6 @@ private CompositeAggregationBuilder createCompositeBuilder(RollupJobConfig confi } if (groupConfig.getTerms() != null) { builders.addAll(groupConfig.getTerms().toBuilders()); - metadata.putAll(groupConfig.getTerms().getMetadata()); } } diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java index d88042188a0fb..376ba00b063d8 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.xpack.core.rollup.job.HistoGroupConfig; import org.elasticsearch.xpack.core.rollup.job.MetricConfig; import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; +import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import org.joda.time.DateTimeZone; import java.util.Arrays; @@ -297,7 +298,7 @@ public void testComparableNoTermsVsTerms() { GroupConfig.Builder group2 = ConfigTestHelpers.getGroupConfig(); group2.setDateHisto(new DateHistoGroupConfig.Builder().setField("foo").setInterval(new DateHistogramInterval("1h")).build()) .setHisto(null) - .setTerms(ConfigTestHelpers.getTerms().setFields(Collections.singletonList("bar")).build()); + .setTerms(new TermsGroupConfig("bar")); job2.setGroupConfig(group2.build()); RollupJobCaps cap2 = new RollupJobCaps(job2.build()); @@ -467,7 +468,7 @@ public void testNoMatchingHistoInterval() { .field("bar") .subAggregation(new MaxAggregationBuilder("the_max").field("max_field")) .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field")); - + RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo") .setGroupConfig(ConfigTestHelpers.getGroupConfig() .setDateHisto(new DateHistoGroupConfig.Builder() diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java index 60c80a065cb66..c2eec92c41080 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java @@ -55,6 +55,7 @@ import org.elasticsearch.xpack.core.rollup.job.DateHistoGroupConfig; import org.elasticsearch.xpack.core.rollup.job.GroupConfig; import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; +import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import org.elasticsearch.xpack.rollup.Rollup; import org.hamcrest.core.IsEqual; import org.joda.time.DateTimeZone; @@ -158,7 +159,7 @@ public void testRangeWrongTZ() { public void testTermQuery() { RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo"); GroupConfig.Builder group = ConfigTestHelpers.getGroupConfig(); - group.setTerms(ConfigTestHelpers.getTerms().setFields(Collections.singletonList("foo")).build()); + group.setTerms(new TermsGroupConfig("foo")); job.setGroupConfig(group.build()); RollupJobCaps cap = new RollupJobCaps(job.build()); Set caps = new HashSet<>(); @@ -171,7 +172,7 @@ public void testTermQuery() { public void testTermsQuery() { RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo"); GroupConfig.Builder group = ConfigTestHelpers.getGroupConfig(); - group.setTerms(ConfigTestHelpers.getTerms().setFields(Collections.singletonList("foo")).build()); + group.setTerms(new TermsGroupConfig("foo")); job.setGroupConfig(group.build()); RollupJobCaps cap = new RollupJobCaps(job.build()); Set caps = new HashSet<>(); @@ -217,7 +218,7 @@ public void testAmbiguousResolution() { RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo"); GroupConfig.Builder group = ConfigTestHelpers.getGroupConfig(); group.setDateHisto(new DateHistoGroupConfig.Builder().setField("foo").setInterval(new DateHistogramInterval("1h")).build()); - group.setTerms(ConfigTestHelpers.getTerms().setFields(Collections.singletonList("foo")).build()).build(); + group.setTerms(new TermsGroupConfig("foo")); job.setGroupConfig(group.build()); RollupJobCaps cap = new RollupJobCaps(job.build()); Set caps = new HashSet<>(); diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index e465c7883cfdd..d037bb72e6408 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.rollup.config; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers; import org.elasticsearch.xpack.core.rollup.job.DateHistoGroupConfig; import org.elasticsearch.xpack.core.rollup.job.GroupConfig; import org.elasticsearch.xpack.core.rollup.job.HistoGroupConfig; @@ -13,7 +14,6 @@ import org.elasticsearch.xpack.core.rollup.job.RollupJob; import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; -import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers; import org.joda.time.DateTimeZone; import java.util.Arrays; @@ -59,7 +59,7 @@ public void testEmptyGroup() { public void testNoDateHisto() { GroupConfig.Builder groupConfig = new GroupConfig.Builder(); - groupConfig.setTerms(ConfigTestHelpers.getTerms().build()); + groupConfig.setTerms(ConfigTestHelpers.randomTermsGroupConfig(random())); groupConfig.setHisto(ConfigTestHelpers.getHisto().build()); Exception e = expectThrows(IllegalArgumentException.class, groupConfig::build); @@ -223,14 +223,9 @@ public void testBadHistoIntervals() { } public void testEmptyTermsField() { - TermsGroupConfig.Builder config = ConfigTestHelpers.getTerms(); - config.setFields(null); - Exception e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [fields] must have at least one value.")); - - config.setFields(Collections.emptyList()); - e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [fields] must have at least one value.")); + final String[] fields = randomBoolean() ? new String[0] : null; + Exception e = expectThrows(IllegalArgumentException.class, () -> new TermsGroupConfig(fields)); + assertThat(e.getMessage(), equalTo("Fields must have at least one value")); } public void testNoHeadersInJSON() { diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java index 07ad0af7f1c38..29cda6ece7503 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java @@ -431,7 +431,7 @@ public void testMissingBuckets() throws IOException { metricFieldType.setName(metricField); // Setup the composite agg - TermsGroupConfig termsGroupConfig = new TermsGroupConfig.Builder().setFields(Collections.singletonList(valueField)).build(); + TermsGroupConfig termsGroupConfig = new TermsGroupConfig(valueField); CompositeAggregationBuilder compositeBuilder = new CompositeAggregationBuilder(RollupIndexer.AGGREGATION_NAME, termsGroupConfig.toBuilders()).size(numDocs*2);