[Rollup] Remove builders from TermsGroupConfig (#32507)

While working on adding the Create Rollup Job API to the 
high level REST client (#29827), I noticed that the configuration 
objects like TermsGroupConfig rely on the Builder pattern in 
order to create or parse instances. These builders are doing 
some validation but the same validation could be done within 
the constructor itself or on the server side when appropriate.

This commit removes the builder for TermsGroupConfig, 
removes some other methods that I consider not really usefull 
once the TermsGroupConfig object will be exposed in the 
high level REST client. It also simplifies the parsing logic.

Related to #29827

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java

@@ -23,6 +23,8 @@ import java.util.Map;
 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 class GroupConfig implements Writeable, ToXContentObject {
             fields.addAll(histo.getAllFields());
         }
         if (terms != null) {
-            fields.addAll(terms.getAllFields());
+            fields.addAll(asList(terms.getFields()));
         }
         return fields;
     }
@@ -100,7 +102,6 @@ public class GroupConfig implements Writeable, ToXContentObject {
         }
     }
 
-
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
@@ -113,9 +114,7 @@ public class GroupConfig implements Writeable, ToXContentObject {
             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 class GroupConfig implements Writeable, ToXContentObject {
             return new GroupConfig(dateHisto, histo, terms);
         }
     }
-}
+}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfig.java

@@ -12,9 +12,10 @@ import org.elasticsearch.common.Strings;
 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 org.elasticsearch.xpack.core.rollup.RollupField;
 
 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 @@ import java.util.stream.Collectors;
  *     ]
  * }
  */
-public class TermsGroupConfig implements Writeable, ToXContentFragment {
-    private static final String NAME = "term_group_config";
-    public static final ObjectParser<TermsGroupConfig.Builder, Void> 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<String> FLOAT_TYPES = Arrays.asList("half_float", "float", "double", "scaled_float");
     private static final List<String> NATURAL_TYPES = Arrays.asList("byte", "short", "integer", "long");
-    private final String[] fields;
 
+    private static final ConstructingObjectParser<TermsGroupConfig, Void> PARSER;
     static {
-        PARSER.declareStringArray(TermsGroupConfig.Builder::setFields, FIELDS);
+        PARSER = new ConstructingObjectParser<>(NAME, args -> {
+            @SuppressWarnings("unchecked") List<String> fields = (List<String>) 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 @@ public class TermsGroupConfig implements Writeable, ToXContentFragment {
         fields = in.readStringArray();
     }
 
+    /**
+     * @return the names of the fields. Never {@code null}.
+     */
     public String[] getFields() {
         return fields;
     }
@@ -72,10 +85,6 @@ public class TermsGroupConfig implements Writeable, ToXContentFragment {
      * set of date histograms.  Used by the rollup indexer to iterate over historical data
      */
     public List<CompositeValuesSourceBuilder<?>> 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 class TermsGroupConfig implements Writeable, ToXContentFragment {
         return map;
     }
 
-    public Map<String, Object> getMetadata() {
-        return Collections.emptyMap();
-    }
-
-    public Set<String> getAllFields() {
-        return Arrays.stream(fields).collect(Collectors.toSet());
-    }
-
     public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCapsResponse,
                                  ActionRequestValidationException validationException) {
 
@@ -138,8 +139,11 @@ public class TermsGroupConfig implements Writeable, ToXContentFragment {
 
     @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 class TermsGroupConfig implements Writeable, ToXContentFragment {
     }
 
     @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 class TermsGroupConfig implements Writeable, ToXContentFragment {
         return Strings.toString(this, true, true);
     }
 
-    public static class Builder {
-        private List<String> fields;
-
-        public List<String> getFields() {
-            return fields;
-        }
-
-        public TermsGroupConfig.Builder setFields(List<String> 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);
     }
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java

@@ -17,9 +17,13 @@ import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig;
 
 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 class ConfigTestHelpers {
             groupBuilder.setHisto(getHisto().build());
         }
         if (ESTestCase.randomBoolean()) {
-            groupBuilder.setTerms(getTerms().build());
+            groupBuilder.setTerms(randomTermsGroupConfig(ESTestCase.random()));
         }
         return groupBuilder;
     }
@@ -105,12 +109,6 @@ public class ConfigTestHelpers {
         return histoBuilder;
     }
 
-    public static TermsGroupConfig.Builder getTerms() {
-        TermsGroupConfig.Builder builder = new TermsGroupConfig.Builder();
-        builder.setFields(getFields());
-        return builder;
-    }
-
     public static  List<String> getFields() {
         return IntStream.range(0, ESTestCase.randomIntBetween(1, 10))
                 .mapToObj(n -> ESTestCase.randomAlphaOfLengthBetween(5, 10))
@@ -126,4 +124,21 @@ public class ConfigTestHelpers {
                 " ?"                                                                         + //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);
+    }
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfigSerializingTests.java

@@ -11,27 +11,23 @@ import org.elasticsearch.common.io.stream.Writeable;
 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<TermsGroupConfig> {
 
-    private static final List<String> FLOAT_TYPES = Arrays.asList("half_float", "float", "double", "scaled_float");
-    private static final List<String> 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 @@ public class TermsGroupConfigSerializingTests extends AbstractSerializingTestCas
 
     @Override
     protected TermsGroupConfig createTestInstance() {
-        return ConfigTestHelpers.getTerms().build();
+        return randomTermsGroupConfig(random());
     }
 
-    public void testValidateNoMapping() throws IOException {
+    public void testValidateNoMapping() {
         ActionRequestValidationException e = new ActionRequestValidationException();
         Map<String, Map<String, FieldCapabilities>> 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<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
 
@@ -65,16 +58,13 @@ public class TermsGroupConfigSerializingTests extends AbstractSerializingTestCas
         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<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
 
@@ -82,17 +72,13 @@ public class TermsGroupConfigSerializingTests extends AbstractSerializingTestCas
         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<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
 
@@ -101,14 +87,12 @@ public class TermsGroupConfigSerializingTests extends AbstractSerializingTestCas
         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<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
         String type = getRandomType();
@@ -118,9 +102,7 @@ public class TermsGroupConfigSerializingTests extends AbstractSerializingTestCas
         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());

x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java

@@ -391,7 +391,6 @@ public abstract class RollupIndexer {
             }
             if (groupConfig.getTerms() != null) {
                 builders.addAll(groupConfig.getTerms().toBuilders());
-                metadata.putAll(groupConfig.getTerms().getMetadata());
             }
         }
 

x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java

@@ -22,6 +22,7 @@ import org.elasticsearch.xpack.core.rollup.job.GroupConfig;
 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 class RollupJobIdentifierUtilTests extends ESTestCase {
         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 class RollupJobIdentifierUtilTests extends ESTestCase {
                 .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()

x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java

@@ -55,6 +55,7 @@ import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps;
 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 class SearchActionTests extends ESTestCase {
     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<RollupJobCaps> caps = new HashSet<>();
@@ -171,7 +172,7 @@ public class SearchActionTests extends ESTestCase {
     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<RollupJobCaps> caps = new HashSet<>();
@@ -217,7 +218,7 @@ public class SearchActionTests extends ESTestCase {
         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<RollupJobCaps> caps = new HashSet<>();

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.MetricConfig;
 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 class ConfigTests extends ESTestCase {
 
     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 class ConfigTests extends ESTestCase {
     }
 
     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() {

x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java

@@ -431,7 +431,7 @@ public class IndexerUtilsTests extends AggregatorTestCase {
         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);