Browse Source

[Rollup] Remove builders from RollupJobConfig (#32669)

Tanguy Leroux 7 years ago
parent
commit
2e65bac5dd
25 changed files with 533 additions and 733 deletions
  1. 1 1
      x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/GetRollupJobsAction.java
  2. 2 3
      x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/PutRollupJobAction.java
  3. 1 1
      x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java
  4. 1 1
      x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJob.java
  5. 126 258
      x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfig.java
  6. 40 20
      x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java
  7. 1 1
      x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/JobWrapperSerializingTests.java
  8. 138 32
      x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfigTests.java
  9. 3 3
      x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/RollupJobTests.java
  10. 2 2
      x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/RollupIndexCaps.java
  11. 1 1
      x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/rest/RestPutRollupJobAction.java
  12. 80 96
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java
  13. 0 18
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java
  14. 4 4
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/GetJobsActionRequestTests.java
  15. 4 4
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/GetRollupCapsActionRequestTests.java
  16. 4 11
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/GetRollupIndexCapsActionRequestTests.java
  17. 5 4
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobActionRequestTests.java
  18. 15 15
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java
  19. 4 4
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/RollupIndexCapsTests.java
  20. 63 86
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java
  21. 4 4
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/TransportTaskHelperTests.java
  22. 2 113
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java
  23. 2 9
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java
  24. 12 24
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerStateTests.java
  25. 18 18
      x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupJobTaskTests.java

+ 1 - 1
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/GetRollupJobsAction.java

@@ -212,7 +212,7 @@ public class GetRollupJobsAction extends Action<GetRollupJobsAction.Response> {
                 (RollupJobStats) a[1], (RollupJobStatus)a[2]));
 
         static {
-            PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> RollupJobConfig.PARSER.apply(p,c).build(), CONFIG);
+            PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> RollupJobConfig.fromXContent(p, null), CONFIG);
             PARSER.declareObject(ConstructingObjectParser.constructorArg(), RollupJobStats.PARSER::apply, STATS);
             PARSER.declareObject(ConstructingObjectParser.constructorArg(), RollupJobStatus.PARSER::apply, STATUS);
         }

+ 2 - 3
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/PutRollupJobAction.java

@@ -52,9 +52,8 @@ public class PutRollupJobAction extends Action<PutRollupJobAction.Response> {
 
         }
 
-        public static Request parseRequest(String id, XContentParser parser) {
-            RollupJobConfig.Builder config = RollupJobConfig.Builder.fromXContent(id, parser);
-            return new Request(config.build());
+        public static Request fromXContent(final XContentParser parser, final String id) throws IOException {
+            return new Request(RollupJobConfig.fromXContent(parser, id));
         }
 
         public RollupJobConfig getConfig() {

+ 1 - 1
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java

@@ -60,7 +60,7 @@ public class MetricConfig implements Writeable, ToXContentObject {
     private static final ParseField AVG = new ParseField("avg");
     private static final ParseField VALUE_COUNT = new ParseField("value_count");
 
-    private static final String NAME = "metrics";
+    static final String NAME = "metrics";
     private static final String FIELD = "field";
     private static final String METRICS = "metrics";
     private static final ConstructingObjectParser<MetricConfig, Void> PARSER;

+ 1 - 1
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJob.java

@@ -41,7 +41,7 @@ public class RollupJob extends AbstractDiffable<RollupJob> implements XPackPlugi
             = new ConstructingObjectParser<>(NAME, a -> new RollupJob((RollupJobConfig) a[0], (Map<String, String>) a[1]));
 
     static {
-        PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> RollupJobConfig.PARSER.apply(p,c).build(), CONFIG);
+        PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> RollupJobConfig.fromXContent(p, null), CONFIG);
         PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.mapStrings(), HEADERS);
     }
 

+ 126 - 258
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfig.java

@@ -7,20 +7,19 @@ package org.elasticsearch.xpack.core.rollup.job;
 
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.fieldcaps.FieldCapabilities;
+import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.NamedWriteable;
 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.regex.Regex;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser;
-import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.xpack.core.rollup.RollupField;
 import org.elasticsearch.xpack.core.scheduler.Cron;
 
 import java.io.IOException;
@@ -30,61 +29,112 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.stream.Collectors;
+
+import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
+import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
 
 /**
  * This class holds the configuration details of a rollup job, such as the groupings, metrics, what
  * index to rollup and where to roll them to.
  */
 public class RollupJobConfig implements NamedWriteable, ToXContentObject {
-    private static final String NAME = "xpack/rollup/jobconfig";
-
-    public static final ParseField TIMEOUT = new ParseField("timeout");
-    public static final ParseField CURRENT = new ParseField("current");
-    public static final ParseField CRON = new ParseField("cron");
-    public static final ParseField PAGE_SIZE = new ParseField("page_size");
-
-    private static final ParseField INDEX_PATTERN = new ParseField("index_pattern");
-    private static final ParseField ROLLUP_INDEX = new ParseField("rollup_index");
-    private static final ParseField GROUPS = new ParseField("groups");
-    private static final ParseField METRICS = new ParseField("metrics");
-
-    private String id;
-    private String indexPattern;
-    private String rollupIndex;
-    private GroupConfig groupConfig;
-    private List<MetricConfig> metricsConfig = Collections.emptyList();
-    private TimeValue timeout = TimeValue.timeValueSeconds(20);
-    private String cron;
-    private int pageSize;
-
-    public static final ObjectParser<RollupJobConfig.Builder, Void> PARSER = new ObjectParser<>(NAME, false, RollupJobConfig.Builder::new);
 
+    private static final String NAME = "xpack/rollup/jobconfig";
+    private static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(20);
+    private static final String ID = "id";
+    private static final String TIMEOUT = "timeout";
+    private static final String CRON = "cron";
+    private static final String PAGE_SIZE = "page_size";
+    private static final String INDEX_PATTERN = "index_pattern";
+    private static final String ROLLUP_INDEX = "rollup_index";
+
+    private final String id;
+    private final String indexPattern;
+    private final String rollupIndex;
+    private final GroupConfig groupConfig;
+    private final List<MetricConfig> metricsConfig;
+    private final TimeValue timeout;
+    private final String cron;
+    private final int pageSize;
+
+    private static final ConstructingObjectParser<RollupJobConfig, String> PARSER;
     static {
-        PARSER.declareString(RollupJobConfig.Builder::setId, RollupField.ID);
-        PARSER.declareObject(RollupJobConfig.Builder::setGroupConfig, (p, c) -> GroupConfig.fromXContent(p), GROUPS);
-        PARSER.declareObjectArray(RollupJobConfig.Builder::setMetricsConfig, (p, c) -> MetricConfig.fromXContent(p), METRICS);
-        PARSER.declareString((params, val) ->
-                params.setTimeout(TimeValue.parseTimeValue(val, TIMEOUT.getPreferredName())), TIMEOUT);
-        PARSER.declareString(RollupJobConfig.Builder::setIndexPattern, INDEX_PATTERN);
-        PARSER.declareString(RollupJobConfig.Builder::setRollupIndex, ROLLUP_INDEX);
-        PARSER.declareString(RollupJobConfig.Builder::setCron, CRON);
-        PARSER.declareInt(RollupJobConfig.Builder::setPageSize, PAGE_SIZE);
+        PARSER = new ConstructingObjectParser<>(NAME, false, (args, optionalId) -> {
+            String id = args[0] != null ? (String) args[0] : optionalId;
+            String indexPattern = (String) args[1];
+            String rollupIndex = (String) args[2];
+            GroupConfig groupConfig = (GroupConfig) args[3];
+            @SuppressWarnings("unchecked")
+            List<MetricConfig> metricsConfig = (List<MetricConfig>) args[4];
+            TimeValue timeout = (TimeValue) args[5];
+            String cron = (String) args[6];
+            int pageSize = (int) args[7];
+            return new RollupJobConfig(id, indexPattern, rollupIndex, cron, pageSize, groupConfig, metricsConfig, timeout);
+        });
+        PARSER.declareString(optionalConstructorArg(), new ParseField(ID));
+        PARSER.declareString(constructorArg(), new ParseField(INDEX_PATTERN));
+        PARSER.declareString(constructorArg(), new ParseField(ROLLUP_INDEX));
+        PARSER.declareObject(optionalConstructorArg(), (p, c) -> GroupConfig.fromXContent(p), new ParseField(GroupConfig.NAME));
+        PARSER.declareObjectArray(optionalConstructorArg(), (p, c) -> MetricConfig.fromXContent(p), new ParseField(MetricConfig.NAME));
+        PARSER.declareField(optionalConstructorArg(), (p, c) -> TimeValue.parseTimeValue(p.textOrNull(), TIMEOUT),
+            new ParseField(TIMEOUT), ObjectParser.ValueType.STRING_OR_NULL);
+        PARSER.declareString(constructorArg(), new ParseField(CRON));
+        PARSER.declareInt(constructorArg(), new ParseField(PAGE_SIZE));
     }
 
-    RollupJobConfig(String id, String indexPattern, String rollupIndex, String cron, int pageSize, GroupConfig groupConfig,
-                    List<MetricConfig> metricsConfig, TimeValue timeout) {
+    public RollupJobConfig(final String id,
+                           final String indexPattern,
+                           final String rollupIndex,
+                           final String cron,
+                           final int pageSize,
+                           final GroupConfig groupConfig,
+                           final List<MetricConfig> metricsConfig,
+                           final @Nullable TimeValue timeout) {
+        if (id == null || id.isEmpty()) {
+            throw new IllegalArgumentException("Id must be a non-null, non-empty string");
+        }
+        if (indexPattern == null || indexPattern.isEmpty()) {
+            throw new IllegalArgumentException("Index pattern must be a non-null, non-empty string");
+        }
+        if (Regex.isMatchAllPattern(indexPattern)) {
+            throw new IllegalArgumentException("Index pattern must not match all indices (as it would match it's own rollup index");
+        }
+        if (Regex.isSimpleMatchPattern(indexPattern)) {
+            if (Regex.simpleMatch(indexPattern, rollupIndex)) {
+                throw new IllegalArgumentException("Index pattern would match rollup index name which is not allowed");
+            }
+        }
+        if (indexPattern.equals(rollupIndex)) {
+            throw new IllegalArgumentException("Rollup index may not be the same as the index pattern");
+        }
+        if (rollupIndex == null || rollupIndex.isEmpty()) {
+            throw new IllegalArgumentException("Rollup index must be a non-null, non-empty string");
+        }
+        if (cron == null || cron.isEmpty()) {
+            throw new IllegalArgumentException("Cron schedule must be a non-null, non-empty string");
+        }
+        if (pageSize <= 0) {
+            throw new IllegalArgumentException("Page size is mandatory and  must be a positive long");
+        }
+        // Cron doesn't have a parse helper method to see if the cron is valid,
+        // so just construct a temporary cron object and if the cron is bad, it'll
+        // throw an exception
+        Cron testCron = new Cron(cron);
+        if (groupConfig == null && (metricsConfig == null || metricsConfig.isEmpty())) {
+            throw new IllegalArgumentException("At least one grouping or metric must be configured");
+        }
+
         this.id = id;
         this.indexPattern = indexPattern;
         this.rollupIndex = rollupIndex;
         this.groupConfig = groupConfig;
-        this.metricsConfig = metricsConfig;
-        this.timeout = timeout;
+        this.metricsConfig = metricsConfig != null ? metricsConfig : Collections.emptyList();
+        this.timeout = timeout != null ? timeout : DEFAULT_TIMEOUT;
         this.cron = cron;
         this.pageSize = pageSize;
     }
 
-    public RollupJobConfig(StreamInput in) throws IOException {
+    public RollupJobConfig(final StreamInput in) throws IOException {
         id = in.readString();
         indexPattern = in.readString();
         rollupIndex = in.readString();
@@ -95,8 +145,6 @@ public class RollupJobConfig implements NamedWriteable, ToXContentObject {
         pageSize = in.readInt();
     }
 
-    public RollupJobConfig() {}
-
     public String getId() {
         return id;
     }
@@ -135,13 +183,20 @@ public class RollupJobConfig implements NamedWriteable, ToXContentObject {
     }
 
     public Set<String> getAllFields() {
-        Set<String> fields = new HashSet<>(groupConfig.getAllFields());
-        fields.addAll(metricsConfig.stream().map(MetricConfig::getField).collect(Collectors.toSet()));
-        return fields;
+        final Set<String> fields = new HashSet<>();
+        if (groupConfig != null) {
+            fields.addAll(groupConfig.getAllFields());
+        }
+        if (metricsConfig != null) {
+            for (MetricConfig metric : metricsConfig) {
+                fields.add(metric.getField());
+            }
+        }
+        return Collections.unmodifiableSet(fields);
     }
 
-    public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCapsResponse,
-                                                             ActionRequestValidationException validationException) {
+    public void validateMappings(final Map<String, Map<String, FieldCapabilities>> fieldCapsResponse,
+                                 final ActionRequestValidationException validationException) {
         groupConfig.validateMappings(fieldCapsResponse, validationException);
         for (MetricConfig m : metricsConfig) {
             m.validateMappings(fieldCapsResponse, validationException);
@@ -149,32 +204,34 @@ public class RollupJobConfig implements NamedWriteable, ToXContentObject {
     }
 
     @Override
-    public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
+    public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException {
         builder.startObject();
-        builder.field(RollupField.ID.getPreferredName(), id);
-        builder.field(INDEX_PATTERN.getPreferredName(), indexPattern);
-        builder.field(ROLLUP_INDEX.getPreferredName(), rollupIndex);
-        builder.field(CRON.getPreferredName(), cron);
-        if (groupConfig != null) {
-            builder.field(GROUPS.getPreferredName(), groupConfig);
-        }
-        if (metricsConfig != null) {
-            builder.startArray(METRICS.getPreferredName());
-            for (MetricConfig metric : metricsConfig) {
-                metric.toXContent(builder, params);
+        {
+            builder.field(ID, id);
+            builder.field(INDEX_PATTERN, indexPattern);
+            builder.field(ROLLUP_INDEX, rollupIndex);
+            builder.field(CRON, cron);
+            if (groupConfig != null) {
+                builder.field(GroupConfig.NAME, groupConfig);
             }
-            builder.endArray();
-        }
-        if (timeout != null) {
-            builder.field(TIMEOUT.getPreferredName(), timeout);
+            if (metricsConfig != null) {
+                builder.startArray(MetricConfig.NAME);
+                for (MetricConfig metric : metricsConfig) {
+                    metric.toXContent(builder, params);
+                }
+                builder.endArray();
+            }
+            if (timeout != null) {
+                builder.field(TIMEOUT, timeout);
+            }
+            builder.field(PAGE_SIZE, pageSize);
         }
-        builder.field(PAGE_SIZE.getPreferredName(), pageSize);
         builder.endObject();
         return builder;
     }
 
     @Override
-    public void writeTo(StreamOutput out) throws IOException {
+    public void writeTo(final StreamOutput out) throws IOException {
         out.writeString(id);
         out.writeString(indexPattern);
         out.writeString(rollupIndex);
@@ -190,13 +247,11 @@ public class RollupJobConfig implements NamedWriteable, ToXContentObject {
         if (this == other) {
             return true;
         }
-
         if (other == null || getClass() != other.getClass()) {
             return false;
         }
 
-        RollupJobConfig that = (RollupJobConfig) other;
-
+        final RollupJobConfig that = (RollupJobConfig) other;
         return Objects.equals(this.id, that.id)
                 && Objects.equals(this.indexPattern, that.indexPattern)
                 && Objects.equals(this.rollupIndex, that.rollupIndex)
@@ -209,8 +264,7 @@ public class RollupJobConfig implements NamedWriteable, ToXContentObject {
 
     @Override
     public int hashCode() {
-        return Objects.hash(id, indexPattern, rollupIndex, cron, groupConfig,
-                metricsConfig, timeout, pageSize);
+        return Objects.hash(id, indexPattern, rollupIndex, cron, groupConfig, metricsConfig, timeout, pageSize);
     }
 
     @Override
@@ -225,193 +279,7 @@ public class RollupJobConfig implements NamedWriteable, ToXContentObject {
         return toString();
     }
 
-    public static class Builder implements Writeable, ToXContentObject {
-        private String id;
-        private String indexPattern;
-        private String rollupIndex;
-        private GroupConfig groupConfig;
-        private List<MetricConfig> metricsConfig = Collections.emptyList();
-        private TimeValue timeout = TimeValue.timeValueSeconds(20);
-        private String cron;
-        private int pageSize = 0;
-
-        public Builder(RollupJobConfig job) {
-            this.id = job.getId();
-            this.indexPattern = job.getIndexPattern();
-            this.rollupIndex = job.getRollupIndex();
-            this.groupConfig = job.getGroupConfig();
-            this.metricsConfig = job.getMetricsConfig();
-            this.timeout = job.getTimeout();
-            this.cron = job.getCron();
-            this.pageSize = job.getPageSize();
-        }
-
-        public static RollupJobConfig.Builder fromXContent(String id, XContentParser parser) {
-            RollupJobConfig.Builder config = RollupJobConfig.PARSER.apply(parser, null);
-            if (id != null) {
-                config.setId(id);
-            }
-            return config;
-        }
-
-        public Builder() {}
-
-        public String getId() {
-            return id;
-        }
-
-        public RollupJobConfig.Builder setId(String id) {
-            this.id = id;
-            return this;
-        }
-
-        public String getIndexPattern() {
-            return indexPattern;
-        }
-
-        public RollupJobConfig.Builder setIndexPattern(String indexPattern) {
-            this.indexPattern = indexPattern;
-            return this;
-        }
-
-        public String getRollupIndex() {
-            return rollupIndex;
-        }
-
-        public RollupJobConfig.Builder setRollupIndex(String rollupIndex) {
-            this.rollupIndex = rollupIndex;
-            return this;
-        }
-
-        public GroupConfig getGroupConfig() {
-            return groupConfig;
-        }
-
-        public RollupJobConfig.Builder setGroupConfig(GroupConfig groupConfig) {
-            this.groupConfig = groupConfig;
-            return this;
-        }
-
-        public List<MetricConfig> getMetricsConfig() {
-            return metricsConfig;
-        }
-
-        public RollupJobConfig.Builder setMetricsConfig(List<MetricConfig> metricsConfig) {
-            this.metricsConfig = metricsConfig;
-            return this;
-        }
-
-        public TimeValue getTimeout() {
-            return timeout;
-        }
-
-        public RollupJobConfig.Builder setTimeout(TimeValue timeout) {
-            this.timeout = timeout;
-            return this;
-        }
-
-        public String getCron() {
-            return cron;
-        }
-
-        public RollupJobConfig.Builder setCron(String cron) {
-            this.cron = cron;
-            return this;
-        }
-
-        public int getPageSize() {
-            return pageSize;
-        }
-
-        public RollupJobConfig.Builder setPageSize(int pageSize) {
-            this.pageSize = pageSize;
-            return this;
-        }
-
-        public RollupJobConfig build() {
-            if (id == null || id.isEmpty()) {
-                throw new IllegalArgumentException("An ID is mandatory.");
-            }
-            if (indexPattern == null || indexPattern.isEmpty()) {
-                throw new IllegalArgumentException("An index pattern is mandatory.");
-            }
-            if (Regex.isMatchAllPattern(indexPattern)) {
-                throw new IllegalArgumentException("Index pattern must not match all indices (as it would match it's own rollup index");
-            }
-            if (Regex.isSimpleMatchPattern(indexPattern)) {
-                if (Regex.simpleMatch(indexPattern, rollupIndex)) {
-                    throw new IllegalArgumentException("Index pattern would match rollup index name which is not allowed.");
-                }
-            }
-            if (indexPattern.equals(rollupIndex)) {
-                throw new IllegalArgumentException("Rollup index may not be the same as the index pattern.");
-            }
-            if (rollupIndex == null || rollupIndex.isEmpty()) {
-                throw new IllegalArgumentException("A rollup index name is mandatory.");
-            }
-            if (cron == null || cron.isEmpty()) {
-                throw new IllegalArgumentException("A cron schedule is mandatory.");
-            }
-            if (pageSize <= 0) {
-                throw new IllegalArgumentException("Parameter [" + PAGE_SIZE.getPreferredName()
-                        + "] is mandatory and  must be a positive long.");
-            }
-            // Cron doesn't have a parse helper method to see if the cron is valid,
-            // so just construct a temporary cron object and if the cron is bad, it'll
-            // throw an exception
-            Cron testCron = new Cron(cron);
-            if (groupConfig == null && (metricsConfig == null || metricsConfig.isEmpty())) {
-                throw new IllegalArgumentException("At least one grouping or metric must be configured.");
-            }
-            return new RollupJobConfig(id, indexPattern, rollupIndex, cron, pageSize, groupConfig,
-                    metricsConfig, timeout);
-        }
-
-        @Override
-        public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-            builder.startObject();
-            if (id != null) {
-                builder.field(RollupField.ID.getPreferredName(), id);
-            }
-            if (indexPattern != null) {
-                builder.field(INDEX_PATTERN.getPreferredName(), indexPattern);
-            }
-            if (indexPattern != null) {
-                builder.field(ROLLUP_INDEX.getPreferredName(), rollupIndex);
-            }
-            if (cron != null) {
-                builder.field(CRON.getPreferredName(), cron);
-            }
-            if (groupConfig != null) {
-                builder.field(GROUPS.getPreferredName(), groupConfig);
-            }
-            if (metricsConfig != null) {
-                builder.startArray(METRICS.getPreferredName());
-                for (MetricConfig config : metricsConfig) {
-                    builder.startObject();
-                    config.toXContent(builder, params);
-                    builder.endObject();
-                }
-                builder.endArray();
-            }
-            if (timeout != null) {
-                builder.field(TIMEOUT.getPreferredName(), timeout);
-            }
-            builder.field(PAGE_SIZE.getPreferredName(), pageSize);
-            builder.endObject();
-            return builder;
-        }
-
-        @Override
-        public void writeTo(StreamOutput out) throws IOException {
-            out.writeString(id);
-            out.writeOptionalString(indexPattern);
-            out.writeOptionalString(rollupIndex);
-            out.writeOptionalString(cron);
-            out.writeOptionalWriteable(groupConfig);
-            out.writeList(metricsConfig);
-            out.writeTimeValue(timeout);
-            out.writeInt(pageSize);
-        }
+    public static RollupJobConfig fromXContent(final XContentParser parser, @Nullable final String optionalJobId) throws IOException {
+        return PARSER.parse(parser, optionalJobId);
     }
 }

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

@@ -17,32 +17,47 @@ import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig;
 import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Random;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import static com.carrotsearch.randomizedtesting.generators.RandomNumbers.randomIntBetween;
+import static com.carrotsearch.randomizedtesting.generators.RandomPicks.randomFrom;
 import static com.carrotsearch.randomizedtesting.generators.RandomStrings.randomAsciiAlphanumOfLengthBetween;
 import static org.elasticsearch.test.ESTestCase.randomDateTimeZone;
 
 public class ConfigTestHelpers {
 
-    public static RollupJobConfig.Builder getRollupJob(String jobId) {
-        RollupJobConfig.Builder builder = new RollupJobConfig.Builder();
-        builder.setId(jobId);
-        builder.setCron(getCronString());
-        builder.setTimeout(new TimeValue(ESTestCase.randomIntBetween(1,100)));
-        String indexPattern = ESTestCase.randomAlphaOfLengthBetween(1,10);
-        builder.setIndexPattern(indexPattern);
-        builder.setRollupIndex("rollup_" + indexPattern); // to ensure the index pattern != rollup index
-        builder.setGroupConfig(ConfigTestHelpers.randomGroupConfig(ESTestCase.random()));
-        builder.setPageSize(ESTestCase.randomIntBetween(1,10));
-        if (ESTestCase.randomBoolean()) {
-            builder.setMetricsConfig(randomMetricsConfigs(ESTestCase.random()));
-        }
-        return builder;
+    private static final String[] TIME_SUFFIXES = new String[]{"d", "h", "ms", "s", "m"};
+
+    private ConfigTestHelpers() {
+    }
+
+    public static RollupJobConfig randomRollupJobConfig(final Random random) {
+        return randomRollupJobConfig(random, randomAsciiAlphanumOfLengthBetween(random, 5, 20));
+    }
+    public static RollupJobConfig randomRollupJobConfig(final Random random, final String id) {
+        return randomRollupJobConfig(random, id, randomAsciiAlphanumOfLengthBetween(random, 5, 20));
+    }
+
+    public static RollupJobConfig randomRollupJobConfig(final Random random, final String id, final String indexPattern) {
+        return randomRollupJobConfig(random, id, indexPattern, "rollup_" + indexPattern);
+    }
+
+    public static RollupJobConfig randomRollupJobConfig(final Random random,
+                                                        final String id,
+                                                        final String indexPattern,
+                                                        final String rollupIndex) {
+        final String cron = randomCron();
+        final int pageSize = randomIntBetween(random, 1, 10);
+        final TimeValue timeout = random.nextBoolean() ? null : randomTimeout(random);
+        final GroupConfig groups = randomGroupConfig(random);
+        final List<MetricConfig> metrics = random.nextBoolean() ? null : randomMetricsConfigs(random);
+        return new RollupJobConfig(id, indexPattern, rollupIndex, cron, pageSize, groups, metrics, timeout);
     }
 
     public static GroupConfig randomGroupConfig(final Random random) {
@@ -52,11 +67,6 @@ public class ConfigTestHelpers {
         return new GroupConfig(dateHistogram, histogram, terms);
     }
 
-    private static final String[] TIME_SUFFIXES = new String[]{"d", "h", "ms", "s", "m"};
-    public static String randomPositiveTimeValue() {
-        return ESTestCase.randomIntBetween(1, 1000) + ESTestCase.randomFrom(TIME_SUFFIXES);
-    }
-
     public static DateHistogramGroupConfig randomDateHistogramGroupConfig(final Random random) {
         final String field = randomField(random);
         final DateHistogramInterval interval = randomInterval();
@@ -71,7 +81,7 @@ public class ConfigTestHelpers {
                 .collect(Collectors.toList());
     }
 
-    public static String getCronString() {
+    public static String randomCron() {
         return (ESTestCase.randomBoolean() ? "*" : String.valueOf(ESTestCase.randomIntBetween(0, 59)))             + //second
                 " " + (ESTestCase.randomBoolean() ? "*" : String.valueOf(ESTestCase.randomIntBetween(0, 59)))      + //minute
                 " " + (ESTestCase.randomBoolean() ? "*" : String.valueOf(ESTestCase.randomIntBetween(0, 23)))      + //hour
@@ -135,6 +145,10 @@ public class ConfigTestHelpers {
         return randomAsciiAlphanumOfLengthBetween(random, 5, 10);
     }
 
+    private static String randomPositiveTimeValue() {
+        return ESTestCase.randomIntBetween(1, 1000) + ESTestCase.randomFrom(TIME_SUFFIXES);
+    }
+
     public static DateHistogramInterval randomInterval() {
         return new DateHistogramInterval(randomPositiveTimeValue());
     }
@@ -142,4 +156,10 @@ public class ConfigTestHelpers {
     private static long randomInterval(final Random random) {
         return RandomNumbers.randomLongBetween(random, 1L, Long.MAX_VALUE);
     }
+
+    public static TimeValue randomTimeout(final Random random) {
+        return new TimeValue(randomIntBetween(random, 0, 60),
+            randomFrom(random, Arrays.asList(TimeUnit.MILLISECONDS, TimeUnit.SECONDS, TimeUnit.MINUTES)));
+    }
+
 }

+ 1 - 1
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/JobWrapperSerializingTests.java

@@ -39,7 +39,7 @@ public class JobWrapperSerializingTests extends AbstractSerializingTestCase<GetR
             state = IndexerState.ABORTING;
         }
 
-        return new GetRollupJobsAction.JobWrapper(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(),
+        return new GetRollupJobsAction.JobWrapper(ConfigTestHelpers.randomRollupJobConfig(random()),
                 new RollupJobStats(randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()),
                 new RollupJobStatus(state, Collections.emptyMap(), randomBoolean()));
     }

+ 138 - 32
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfigTests.java

@@ -8,17 +8,27 @@ package org.elasticsearch.xpack.core.rollup.job;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.test.AbstractSerializingTestCase;
-import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig;
-import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
+import org.junit.Before;
 
+import java.io.IOException;
+
+import static java.util.Collections.emptyList;
+import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomRollupJobConfig;
 import static org.hamcrest.Matchers.equalTo;
 
 
 public class RollupJobConfigTests extends AbstractSerializingTestCase<RollupJobConfig> {
 
+    private String jobId;
+
+    @Before
+    public void setUpOptionalId() {
+        jobId = randomAlphaOfLengthBetween(1, 10);
+    }
+
     @Override
     protected RollupJobConfig createTestInstance() {
-        return ConfigTestHelpers.getRollupJob(randomAlphaOfLengthBetween(1,10)).build();
+        return randomRollupJobConfig(random(), jobId);
     }
 
     @Override
@@ -27,43 +37,139 @@ public class RollupJobConfigTests extends AbstractSerializingTestCase<RollupJobC
     }
 
     @Override
-    protected RollupJobConfig doParseInstance(XContentParser parser) {
-        return RollupJobConfig.PARSER.apply(parser, null).build();
+    protected RollupJobConfig doParseInstance(final XContentParser parser) throws IOException {
+        if (randomBoolean()) {
+            return RollupJobConfig.fromXContent(parser, jobId);
+        } else {
+            return RollupJobConfig.fromXContent(parser, null);
+        }
     }
 
     public void testEmptyIndexPattern() {
-        RollupJobConfig.Builder builder = ConfigTestHelpers.getRollupJob(randomAlphaOfLengthBetween(1, 10));
-        builder.setIndexPattern(null);
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build);
-        assertThat(e.getMessage(), equalTo("An index pattern is mandatory."));
-
-        builder = ConfigTestHelpers.getRollupJob(randomAlphaOfLengthBetween(1, 10));
-        builder.setIndexPattern("");
-        e = expectThrows(IllegalArgumentException.class, builder::build);
-        assertThat(e.getMessage(), equalTo("An index pattern is mandatory."));
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), null, sample.getRollupIndex(), sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Index pattern must be a non-null, non-empty string"));
+
+        e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), "", sample.getRollupIndex(), sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Index pattern must be a non-null, non-empty string"));
     }
 
     public void testEmptyCron() {
-        RollupJobConfig.Builder builder = ConfigTestHelpers.getRollupJob(randomAlphaOfLengthBetween(1, 10));
-        builder.setCron(null);
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build);
-        assertThat(e.getMessage(), equalTo("A cron schedule is mandatory."));
-
-        builder = ConfigTestHelpers.getRollupJob(randomAlphaOfLengthBetween(1, 10));
-        builder.setCron("");
-        e = expectThrows(IllegalArgumentException.class, builder::build);
-        assertThat(e.getMessage(), equalTo("A cron schedule is mandatory."));
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), sample.getRollupIndex(), null, sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Cron schedule must be a non-null, non-empty string"));
+
+        e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), sample.getRollupIndex(), "", sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Cron schedule must be a non-null, non-empty string"));
     }
 
     public void testEmptyID() {
-        RollupJobConfig.Builder builder = ConfigTestHelpers.getRollupJob(randomAlphaOfLengthBetween(1, 10));
-        builder.setId(null);
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build);
-        assertThat(e.getMessage(), equalTo("An ID is mandatory."));
-
-        builder = ConfigTestHelpers.getRollupJob(randomAlphaOfLengthBetween(1, 10));
-        builder.setId("");
-        e = expectThrows(IllegalArgumentException.class, builder::build);
-        assertThat(e.getMessage(), equalTo("An ID is mandatory."));
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(null, sample.getIndexPattern(), sample.getRollupIndex(), sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Id must be a non-null, non-empty string"));
+
+        e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig("", sample.getIndexPattern(), sample.getRollupIndex(), sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Id must be a non-null, non-empty string"));
+    }
+
+    public void testBadCron() {
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        Exception e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), sample.getRollupIndex(), "0 * * *", sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("invalid cron expression [0 * * *]"));
+    }
+
+    public void testMatchAllIndexPattern() {
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        Exception e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), "*", sample.getRollupIndex(), sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Index pattern must not match all indices (as it would match it's own rollup index"));
+    }
+
+    public void testMatchOwnRollupPatternPrefix() {
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        Exception e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), "foo-*", "foo-rollup", sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Index pattern would match rollup index name which is not allowed"));
+    }
+
+    public void testMatchOwnRollupPatternSuffix() {
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        Exception e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), "*-rollup", "foo-rollup", sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Index pattern would match rollup index name which is not allowed"));
+    }
+
+    public void testIndexPatternIdenticalToRollup() {
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        Exception e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), "foo", "foo", sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Rollup index may not be the same as the index pattern"));
+    }
+
+    public void testEmptyRollupIndex() {
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+        Exception e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), "", sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Rollup index must be a non-null, non-empty string"));
+
+        e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), null, sample.getCron(), sample.getPageSize(),
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Rollup index must be a non-null, non-empty string"));
+    }
+
+    public void testBadSize() {
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        Exception e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), sample.getRollupIndex(), sample.getCron(), -1,
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Page size is mandatory and  must be a positive long"));
+
+        e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), sample.getRollupIndex(), sample.getCron(), 0,
+                sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("Page size is mandatory and  must be a positive long"));
+    }
+
+    public void testEmptyGroupAndMetrics() {
+        final RollupJobConfig sample = randomRollupJobConfig(random());
+
+        Exception e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), sample.getRollupIndex(), sample.getCron(), sample.getPageSize(),
+                null, null, sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("At least one grouping or metric must be configured"));
+
+        e = expectThrows(IllegalArgumentException.class, () ->
+            new RollupJobConfig(sample.getId(), sample.getIndexPattern(), sample.getRollupIndex(), sample.getCron(), sample.getPageSize(),
+                null, emptyList(), sample.getTimeout()));
+        assertThat(e.getMessage(), equalTo("At least one grouping or metric must be configured"));
     }
 }

+ 3 - 3
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/RollupJobTests.java

@@ -37,7 +37,7 @@ public class RollupJobTests extends AbstractDiffableSerializationTestCase {
     @Override
     protected Writeable createTestInstance() {
         if (randomBoolean()) {
-            return new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), null);
+            return new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), null);
         }
 
         Map<String, String> headers = Collections.emptyMap();
@@ -45,7 +45,7 @@ public class RollupJobTests extends AbstractDiffableSerializationTestCase {
             headers = new HashMap<>(1);
             headers.put("foo", "bar");
         }
-        return new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), headers);
+        return new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), headers);
     }
 
     @Override
@@ -60,7 +60,7 @@ public class RollupJobTests extends AbstractDiffableSerializationTestCase {
                 return new RollupJob(other.getConfig(), null);
             }
         } else {
-            return new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), other.getHeaders());
+            return new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), other.getHeaders());
         }
     }
 }

+ 2 - 2
x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/RollupIndexCaps.java

@@ -75,7 +75,7 @@ public class RollupIndexCaps implements Writeable, ToXContentFragment {
 
                         // "job-1"
                         while (parser.nextToken().equals(XContentParser.Token.END_OBJECT) == false) {
-                            jobs.add(RollupJobConfig.PARSER.apply(parser, aVoid).build());
+                            jobs.add(RollupJobConfig.fromXContent(parser, null));
                         }
                     }
                 }
@@ -167,4 +167,4 @@ public class RollupIndexCaps implements Writeable, ToXContentFragment {
     public int hashCode() {
         return Objects.hash(rollupIndexName, jobCaps);
     }
-}
+}

+ 1 - 1
x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/rest/RestPutRollupJobAction.java

@@ -32,7 +32,7 @@ public class RestPutRollupJobAction extends BaseRestHandler {
         String id = restRequest.param(ID.getPreferredName());
         XContentParser parser = restRequest.contentParser();
 
-        PutRollupJobAction.Request request = PutRollupJobAction.Request.parseRequest(id, parser);
+        PutRollupJobAction.Request request = PutRollupJobAction.Request.fromXContent(parser, id);
 
         return channel -> client.execute(PutRollupJobAction.INSTANCE, request, new RestToXContentListener<>(channel));
     }

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

@@ -15,7 +15,6 @@ import org.elasticsearch.search.aggregations.metrics.min.MinAggregationBuilder;
 import org.elasticsearch.search.aggregations.metrics.sum.SumAggregationBuilder;
 import org.elasticsearch.search.aggregations.support.ValueType;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
 import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps;
 import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig;
 import org.elasticsearch.xpack.core.rollup.job.GroupConfig;
@@ -27,18 +26,19 @@ import org.joda.time.DateTimeZone;
 
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
+import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.hamcrest.Matchers.equalTo;
 
 public class RollupJobIdentifierUtilTests extends ESTestCase {
 
     public void testOneMatch() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
         Set<RollupJobCaps> caps = singletonSet(cap);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
@@ -49,10 +49,9 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testBiggerButCompatibleInterval() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
         Set<RollupJobCaps> caps = singletonSet(cap);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
@@ -63,10 +62,9 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testIncompatibleInterval() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1d")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
         Set<RollupJobCaps> caps = singletonSet(cap);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
@@ -78,10 +76,9 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testBadTimeZone() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"), null, "EST"));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
         Set<RollupJobCaps> caps = singletonSet(cap);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
@@ -94,11 +91,10 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testMetricOnlyAgg() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        job.setMetricsConfig(singletonList(new MetricConfig("bar", singletonList("max"))));
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final List<MetricConfig> metrics = singletonList(new MetricConfig("bar", singletonList("max")));
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, metrics, null);
+        RollupJobCaps cap = new RollupJobCaps(job);
         Set<RollupJobCaps> caps = singletonSet(cap);
 
         MaxAggregationBuilder max = new MaxAggregationBuilder("the_max").field("bar");
@@ -108,10 +104,9 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testOneOfTwoMatchingCaps() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
         Set<RollupJobCaps> caps = singletonSet(cap);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
@@ -124,18 +119,16 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testTwoJobsSameRollupIndex() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
         Set<RollupJobCaps> caps = new HashSet<>(2);
         caps.add(cap);
 
-        RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2");
         final GroupConfig group2 = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job2.setGroupConfig(group);
-        job2.setRollupIndex(job.getRollupIndex());
-        RollupJobCaps cap2 = new RollupJobCaps(job2.build());
+        final RollupJobConfig job2 =
+            new RollupJobConfig("foo2", "index", job.getRollupIndex(), "*/5 * * * * ?", 10,  group2, emptyList(), null);
+        RollupJobCaps cap2 = new RollupJobCaps(job2);
         caps.add(cap2);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
@@ -148,19 +141,16 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testTwoJobsButBothPartialMatches() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        job.setMetricsConfig(singletonList(new MetricConfig("bar", singletonList("max"))));
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final List<MetricConfig> metrics = singletonList(new MetricConfig("bar", singletonList("max")));
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, metrics, null);
+        RollupJobCaps cap = new RollupJobCaps(job);
         Set<RollupJobCaps> caps = new HashSet<>(2);
         caps.add(cap);
 
-        RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2");
-        final GroupConfig group2 = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job2.setGroupConfig(group);
-        job.setMetricsConfig(singletonList(new MetricConfig("bar", singletonList("min"))));
-        RollupJobCaps cap2 = new RollupJobCaps(job2.build());
+        // TODO Is it what we really want to test?
+        final RollupJobConfig job2 = new RollupJobConfig("foo2", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap2 = new RollupJobCaps(job2);
         caps.add(cap2);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
@@ -174,15 +164,14 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testComparableDifferentDateIntervals() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
 
-        RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2").setRollupIndex(job.getRollupIndex());
         final GroupConfig group2 = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1d")));
-        job2.setGroupConfig(group2);
-        RollupJobCaps cap2 = new RollupJobCaps(job2.build());
+        final RollupJobConfig job2 =
+            new RollupJobConfig("foo2", "index", job.getRollupIndex(), "*/5 * * * * ?", 10,  group2, emptyList(), null);
+        RollupJobCaps cap2 = new RollupJobCaps(job2);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
                 .dateHistogramInterval(new DateHistogramInterval("1d"));
@@ -197,15 +186,14 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testComparableDifferentDateIntervalsOnlyOneWorks() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
 
-        RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2").setRollupIndex(job.getRollupIndex());
         final GroupConfig group2 = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1d")));
-        job2.setGroupConfig(group2);
-        RollupJobCaps cap2 = new RollupJobCaps(job2.build());
+        final RollupJobConfig job2 =
+            new RollupJobConfig("foo2", "index", job.getRollupIndex(), "*/5 * * * * ?", 10,  group2, emptyList(), null);
+        RollupJobCaps cap2 = new RollupJobCaps(job2);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
                 .dateHistogramInterval(new DateHistogramInterval("1h"));
@@ -220,16 +208,15 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testComparableNoHistoVsHisto() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
 
-        RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2").setRollupIndex(job.getRollupIndex());
         final HistogramGroupConfig histoConfig = new HistogramGroupConfig(100L, "bar");
         final GroupConfig group2 = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")), histoConfig, null);
-        job2.setGroupConfig(group2);
-        RollupJobCaps cap2 = new RollupJobCaps(job2.build());
+        final RollupJobConfig job2 =
+            new RollupJobConfig("foo2", "index", job.getRollupIndex(), "*/5 * * * * ?", 10,  group2, emptyList(), null);
+        RollupJobCaps cap2 = new RollupJobCaps(job2);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
                 .dateHistogramInterval(new DateHistogramInterval("1h"))
@@ -245,16 +232,15 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     }
 
     public void testComparableNoTermsVsTerms() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
 
-        RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2").setRollupIndex(job.getRollupIndex());
         final TermsGroupConfig termsConfig = new TermsGroupConfig("bar");
         final GroupConfig group2 = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")), null, termsConfig);
-        job2.setGroupConfig(group2);
-        RollupJobCaps cap2 = new RollupJobCaps(job2.build());
+        final RollupJobConfig job2 =
+            new RollupJobConfig("foo2", "index", job.getRollupIndex(), "*/5 * * * * ?", 10,  group2, emptyList(), null);
+        RollupJobCaps cap2 = new RollupJobCaps(job2);
 
         DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo")
                 .dateHistogramInterval(new DateHistogramInterval("1h"))
@@ -276,16 +262,16 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
                 .subAggregation(new MaxAggregationBuilder("the_max").field("max_field"))
                 .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field"));
 
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo")
-                .setGroupConfig(new GroupConfig(
+        final GroupConfig group = new GroupConfig(
                     // NOTE same name but wrong type
                     new DateHistogramGroupConfig("foo", new DateHistogramInterval("1d"), null, DateTimeZone.UTC.getID()),
                     new HistogramGroupConfig(1L, "baz"), // <-- NOTE right type but wrong name
                     null
-                ))
-                .setMetricsConfig(
-                    Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg"))))
-                .build();
+                );
+        final List<MetricConfig> metrics =
+            Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg")));
+
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, metrics, null);
         Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(job));
 
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> RollupJobIdentifierUtils.findBestJobs(histo, caps));
@@ -300,13 +286,13 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
                 .subAggregation(new MaxAggregationBuilder("the_max").field("max_field"))
                 .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field"));
 
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo")
-                .setGroupConfig(new GroupConfig(
+        final GroupConfig group = new GroupConfig(
                     new DateHistogramGroupConfig("foo", new DateHistogramInterval("1d"), null, DateTimeZone.UTC.getID())
-                ))
-                .setMetricsConfig(
-                    Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg"))))
-                .build();
+                );
+        final List<MetricConfig> metrics =
+                    Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg")));
+
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, metrics, null);
         Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(job));
 
         Exception e = expectThrows(IllegalArgumentException.class, () -> RollupJobIdentifierUtils.findBestJobs(histo,caps));
@@ -321,12 +307,11 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
                 .subAggregation(new MaxAggregationBuilder("the_max").field("max_field"))
                 .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field"));
 
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo")
-                .setGroupConfig(new GroupConfig(
+        final GroupConfig group = new GroupConfig(
                     // interval in job is much higher than agg interval above
                     new DateHistogramGroupConfig("foo", new DateHistogramInterval("100d"), null, DateTimeZone.UTC.getID())
-                ))
-                .build();
+                );
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
         Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(job));
 
         Exception e = expectThrows(RuntimeException.class, () -> RollupJobIdentifierUtils.findBestJobs(histo, caps));
@@ -341,14 +326,14 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
                 .subAggregation(new MaxAggregationBuilder("the_max").field("max_field"))
                 .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field"));
 
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo")
-                .setGroupConfig(new GroupConfig(
+        final GroupConfig group = new GroupConfig(
                     // NOTE different field from the one in the query
                     new DateHistogramGroupConfig("bar", new DateHistogramInterval("1d"), null, DateTimeZone.UTC.getID())
-                ))
-                .setMetricsConfig(
-                    Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg"))))
-                .build();
+                );
+        final List<MetricConfig> metrics =
+                    Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg")));
+
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, metrics, null);
         Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(job));
 
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> RollupJobIdentifierUtils.findBestJobs(histo, caps));
@@ -363,15 +348,15 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
                 .subAggregation(new MaxAggregationBuilder("the_max").field("max_field"))
                 .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field"));
 
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo")
-                .setGroupConfig(new GroupConfig(
+        final GroupConfig group = new GroupConfig(
                     new DateHistogramGroupConfig("bar", new DateHistogramInterval("1d"), null, DateTimeZone.UTC.getID()),
                     new HistogramGroupConfig(1L, "baz"), // <-- NOTE right type but wrong name
                     null
-                ))
-                .setMetricsConfig(
-                    Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg"))))
-                .build();
+                );
+        final List<MetricConfig> metrics =
+                    Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg")));
+
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, metrics, null);
         Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(job));
 
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> RollupJobIdentifierUtils.findBestJobs(histo, caps));
@@ -386,13 +371,12 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
                 .subAggregation(new MaxAggregationBuilder("the_max").field("max_field"))
                 .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field"));
 
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo")
-                .setGroupConfig(new GroupConfig(
+        final GroupConfig group = new GroupConfig(
                     new DateHistogramGroupConfig("foo", new DateHistogramInterval("1d"), null, DateTimeZone.UTC.getID()),
                     new HistogramGroupConfig(1L, "baz"), // <-- NOTE right type but wrong name
                     null
-                ))
-                .build();
+                );
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
         Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(job));
 
         Exception e = expectThrows(RuntimeException.class,
@@ -404,10 +388,10 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
     public void testMissingMetric() {
         int i = ESTestCase.randomIntBetween(0, 3);
 
-        Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(ConfigTestHelpers
-                .getRollupJob("foo")
-                    .setMetricsConfig(singletonList(new MetricConfig("foo", Arrays.asList("avg", "max", "min", "sum"))))
-                .build()));
+        final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
+        final List<MetricConfig> metrics = singletonList(new MetricConfig("foo", Arrays.asList("avg", "max", "min", "sum")));
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  group, emptyList(), null);
+        Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(job));
 
         String aggType;
         Exception e;

+ 0 - 18
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java

@@ -27,25 +27,18 @@ import org.elasticsearch.search.aggregations.metrics.sum.SumAggregationBuilder;
 import org.elasticsearch.search.aggregations.support.ValueType;
 import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
-import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps;
-import org.elasticsearch.xpack.core.rollup.job.MetricConfig;
 import org.hamcrest.Matchers;
 import org.junit.Before;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import static java.util.Collections.emptyList;
-import static java.util.Collections.singletonList;
 import static org.elasticsearch.xpack.rollup.RollupRequestTranslator.translateAggregation;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.core.IsInstanceOf.instanceOf;
@@ -153,11 +146,6 @@ public class RollupRequestTranslationTests extends ESTestCase {
     }
 
     public void testUnsupportedMetric() {
-        Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(ConfigTestHelpers
-                .getRollupJob("foo")
-                    .setMetricsConfig(singletonList(new MetricConfig("foo", Arrays.asList("avg", "max", "min", "sum"))))
-                .build()));
-
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
                 () -> translateAggregation(new StatsAggregationBuilder("test_metric")
                         .field("foo"), Collections.emptyList(), namedWriteableRegistry));
@@ -384,10 +372,4 @@ public class RollupRequestTranslationTests extends ESTestCase {
         assertThat(e.getMessage(), equalTo("Unable to translate aggregation tree into Rollup.  Aggregation [test_geo] is of type " +
                 "[GeoDistanceAggregationBuilder] which is currently unsupported."));
     }
-
-    private Set<RollupJobCaps> singletonSet(RollupJobCaps cap) {
-        Set<RollupJobCaps> caps = new HashSet<>();
-        caps.add(cap);
-        return caps;
-    }
 }

+ 4 - 4
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/GetJobsActionRequestTests.java

@@ -70,7 +70,7 @@ public class GetJobsActionRequestTests extends AbstractStreamableTestCase<Reques
 
     public void testStateCheckMatchingPersistentTasks() {
         GetRollupJobsAction.Request request = new GetRollupJobsAction.Request("foo");
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random(), "foo"), Collections.emptyMap());
         Map<String, PersistentTasksCustomMetaData.PersistentTask<?>> tasks
                 = Collections.singletonMap("foo", new PersistentTasksCustomMetaData.PersistentTask<>("foo", RollupJob.NAME, job, 1, null));
         ClusterState state = ClusterState.builder(new ClusterName("_name"))
@@ -83,7 +83,7 @@ public class GetJobsActionRequestTests extends AbstractStreamableTestCase<Reques
 
     public void testStateCheckAllMatchingPersistentTasks() {
         GetRollupJobsAction.Request request = new GetRollupJobsAction.Request("_all");
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random(), "foo"), Collections.emptyMap());
         Map<String, PersistentTasksCustomMetaData.PersistentTask<?>> tasks
                 = Collections.singletonMap("foo", new PersistentTasksCustomMetaData.PersistentTask<>("foo", RollupJob.NAME, job, 1, null));
         ClusterState state = ClusterState.builder(new ClusterName("_name"))
@@ -96,8 +96,8 @@ public class GetJobsActionRequestTests extends AbstractStreamableTestCase<Reques
 
     public void testStateCheckAllWithSeveralMatchingPersistentTasks() {
         GetRollupJobsAction.Request request = new GetRollupJobsAction.Request("_all");
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
-        RollupJob job2 = new RollupJob(ConfigTestHelpers.getRollupJob("bar").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random(), "foo"), Collections.emptyMap());
+        RollupJob job2 = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random(), "bar"), Collections.emptyMap());
         Map<String, PersistentTasksCustomMetaData.PersistentTask<?>> tasks = new HashMap<>(2);
         tasks.put("foo", new PersistentTasksCustomMetaData.PersistentTask<>("foo", RollupJob.NAME, job, 1, null));
         tasks.put("bar", new PersistentTasksCustomMetaData.PersistentTask<>("bar", RollupJob.NAME, job2, 1, null));

+ 4 - 4
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/GetRollupCapsActionRequestTests.java

@@ -89,7 +89,7 @@ public class GetRollupCapsActionRequestTests extends AbstractStreamableTestCase<
     public void testOneJob() throws IOException {
         String indexPattern = randomBoolean() ? randomAlphaOfLength(10) : randomAlphaOfLength(10) + "-*";
         String jobName = randomAlphaOfLength(5);
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob(jobName).build();
+        RollupJobConfig job = ConfigTestHelpers.randomRollupJobConfig(random(), jobName);
 
         MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
                 Collections.singletonMap(RollupField.TYPE_NAME,
@@ -113,7 +113,7 @@ public class GetRollupCapsActionRequestTests extends AbstractStreamableTestCase<
         Map<String, Object> jobs = new HashMap<>(num);
         for (int i = 0; i < num; i++) {
             String jobName = randomAlphaOfLength(5);
-            jobs.put(jobName, ConfigTestHelpers.getRollupJob(jobName).build());
+            jobs.put(jobName, ConfigTestHelpers.randomRollupJobConfig(random(), jobName));
         }
 
         MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
@@ -147,7 +147,7 @@ public class GetRollupCapsActionRequestTests extends AbstractStreamableTestCase<
                 String jobName = randomAlphaOfLength(10);
                 String indexName = Integer.toString(indexCounter);
                 indexCounter += 1;
-                jobs.put(jobName, ConfigTestHelpers.getRollupJob(jobName).setIndexPattern(indexName).build());
+                jobs.put(jobName, ConfigTestHelpers.randomRollupJobConfig(random(), jobName, indexName));
             }
 
             MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
@@ -179,7 +179,7 @@ public class GetRollupCapsActionRequestTests extends AbstractStreamableTestCase<
             Map<String, Object> jobs = new HashMap<>(num);
             for (int i = 0; i < num; i++) {
                 String jobName = randomAlphaOfLength(5);
-                jobs.put(jobName, ConfigTestHelpers.getRollupJob(jobName).setIndexPattern(indexName).build());
+                jobs.put(jobName, ConfigTestHelpers.randomRollupJobConfig(random(), jobName, indexName));
             }
 
             MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,

+ 4 - 11
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/GetRollupIndexCapsActionRequestTests.java

@@ -60,7 +60,7 @@ public class GetRollupIndexCapsActionRequestTests extends AbstractStreamableTest
                 String jobName = randomAlphaOfLength(10);
                 String indexName = Integer.toString(indexCounter);
                 indexCounter += 1;
-                jobs.put(jobName, ConfigTestHelpers.getRollupJob(jobName).setRollupIndex("foo").build());
+                jobs.put(jobName, ConfigTestHelpers.randomRollupJobConfig(random(), jobName, indexName, "foo"));
             }
 
             MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
@@ -89,10 +89,7 @@ public class GetRollupIndexCapsActionRequestTests extends AbstractStreamableTest
             String jobName = randomAlphaOfLength(10);
             String indexName = Integer.toString(indexCounter);
             indexCounter += 1;
-            jobs.put(jobName, ConfigTestHelpers.getRollupJob(jobName)
-                .setIndexPattern(indexName)
-                .setRollupIndex("rollup_" + indexName).build());
-
+            jobs.put(jobName, ConfigTestHelpers.randomRollupJobConfig(random(), jobName, indexName, "rollup_" + indexName));
 
             MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
                 Collections.singletonMap(RollupField.TYPE_NAME,
@@ -120,9 +117,7 @@ public class GetRollupIndexCapsActionRequestTests extends AbstractStreamableTest
             String jobName = randomAlphaOfLength(10);
             String indexName = Integer.toString(indexCounter);
             indexCounter += 1;
-            jobs.put(jobName, ConfigTestHelpers.getRollupJob(jobName)
-                .setIndexPattern("foo_" + indexName)
-                .setRollupIndex("rollup_" + indexName).build());
+            jobs.put(jobName, ConfigTestHelpers.randomRollupJobConfig(random(), jobName, "foo_" + indexName, "rollup_" + indexName));
 
             MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
                 Collections.singletonMap(RollupField.TYPE_NAME,
@@ -151,9 +146,7 @@ public class GetRollupIndexCapsActionRequestTests extends AbstractStreamableTest
             String jobName = randomAlphaOfLength(10);
             String indexName = Integer.toString(indexCounter);
             indexCounter += 1;
-            jobs.put(jobName, ConfigTestHelpers.getRollupJob(jobName)
-                .setIndexPattern("foo_" + indexName)
-                .setRollupIndex("rollup_foo").build());
+            jobs.put(jobName, ConfigTestHelpers.randomRollupJobConfig(random(), jobName, "foo_" + indexName, "rollup_foo"));
 
             MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
                 Collections.singletonMap(RollupField.TYPE_NAME,

+ 5 - 4
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobActionRequestTests.java

@@ -12,6 +12,8 @@ import org.elasticsearch.xpack.core.rollup.action.PutRollupJobAction.Request;
 import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
 import org.junit.Before;
 
+import java.io.IOException;
+
 public class PutJobActionRequestTests extends AbstractStreamableXContentTestCase<Request> {
 
     private String jobId;
@@ -23,7 +25,7 @@ public class PutJobActionRequestTests extends AbstractStreamableXContentTestCase
 
     @Override
     protected Request createTestInstance() {
-        return new Request(ConfigTestHelpers.getRollupJob(jobId).build());
+        return new Request(ConfigTestHelpers.randomRollupJobConfig(random(), jobId));
     }
 
     @Override
@@ -37,9 +39,8 @@ public class PutJobActionRequestTests extends AbstractStreamableXContentTestCase
     }
 
     @Override
-    protected Request doParseInstance(XContentParser parser) {
-        return Request.parseRequest(jobId, parser);
+    protected Request doParseInstance(final XContentParser parser) throws IOException {
+        return Request.fromXContent(parser, jobId);
     }
 
-
 }

+ 15 - 15
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java

@@ -50,7 +50,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testCreateIndexException() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random(), "foo"), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
@@ -76,7 +76,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testIndexAlreadyExists() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
@@ -108,7 +108,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testIndexMetaData() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
@@ -151,7 +151,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testGetMappingFails() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random(), "foo"), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
@@ -175,7 +175,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testNoMetadataInMapping() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
@@ -208,7 +208,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testNoMappingVersion() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
@@ -245,7 +245,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testJobAlreadyInMapping() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random(), "foo"), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
@@ -282,12 +282,12 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testAddJobToMapping() {
-        RollupJobConfig unrelatedJob = ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLength(10))
-            .setIndexPattern("foo").setRollupIndex("rollup_index_foo").build();
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo")
-            .setIndexPattern("foo")
-            .setRollupIndex("rollup_index_foo")
-            .build(), Collections.emptyMap());
+        final RollupJobConfig unrelatedJob =
+            ConfigTestHelpers.randomRollupJobConfig(random(), ESTestCase.randomAlphaOfLength(10), "foo", "rollup_index_foo");
+
+        final RollupJobConfig config =
+            ConfigTestHelpers.randomRollupJobConfig(random(), ESTestCase.randomAlphaOfLength(10), "foo", "rollup_index_foo");
+        RollupJob job = new RollupJob(config, Collections.emptyMap());
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
         }, e -> {
@@ -331,7 +331,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testTaskAlreadyExists() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random(), "foo"), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");
@@ -354,7 +354,7 @@ public class PutJobStateMachineTests extends ESTestCase {
 
     @SuppressWarnings("unchecked")
     public void testStartTask() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob("foo").build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
 
         ActionListener<PutRollupJobAction.Response> testListener = ActionListener.wrap(response -> {
             fail("Listener success should not have been triggered.");

+ 4 - 4
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/RollupIndexCapsTests.java

@@ -30,8 +30,8 @@ public class RollupIndexCapsTests extends ESTestCase {
 
     public void testGetAllJobs() {
         List<RollupJobConfig> jobs = new ArrayList<>(2);
-        jobs.add(ConfigTestHelpers.getRollupJob("foo").build());
-        jobs.add(ConfigTestHelpers.getRollupJob("bar").build());
+        jobs.add(ConfigTestHelpers.randomRollupJobConfig(random(), "foo"));
+        jobs.add(ConfigTestHelpers.randomRollupJobConfig(random(), "bar"));
         RollupIndexCaps caps = new RollupIndexCaps(ESTestCase.randomAlphaOfLength(10), jobs);
         assertTrue(caps.hasCaps());
 
@@ -45,8 +45,8 @@ public class RollupIndexCapsTests extends ESTestCase {
 
     public void testFilterGetJobs() {
         List<RollupJobConfig> jobs = new ArrayList<>(2);
-        jobs.add(ConfigTestHelpers.getRollupJob("foo").setIndexPattern("foo_index_pattern").build());
-        jobs.add(ConfigTestHelpers.getRollupJob("bar").build());
+        jobs.add(ConfigTestHelpers.randomRollupJobConfig(random(), "foo", "foo_index_pattern"));
+        jobs.add(ConfigTestHelpers.randomRollupJobConfig(random(), "bar"));
         RollupIndexCaps caps = new RollupIndexCaps(ESTestCase.randomAlphaOfLength(10), jobs);
         assertTrue(caps.hasCaps());
 

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

@@ -16,9 +16,6 @@ import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
-import org.elasticsearch.common.util.BigArrays;
-import org.elasticsearch.common.util.MockBigArrays;
-import org.elasticsearch.common.util.MockPageCacheRecycler;
 import org.elasticsearch.index.query.BoolQueryBuilder;
 import org.elasticsearch.index.query.BoostingQueryBuilder;
 import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
@@ -30,8 +27,6 @@ import org.elasticsearch.index.query.RangeQueryBuilder;
 import org.elasticsearch.index.query.TermQueryBuilder;
 import org.elasticsearch.index.query.TermsQueryBuilder;
 import org.elasticsearch.indices.IndicesModule;
-import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
-import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.SearchModule;
 import org.elasticsearch.search.aggregations.Aggregations;
@@ -54,10 +49,12 @@ import org.elasticsearch.xpack.core.rollup.RollupField;
 import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps;
 import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig;
 import org.elasticsearch.xpack.core.rollup.job.GroupConfig;
+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.elasticsearch.xpack.rollup.Rollup;
 import org.hamcrest.core.IsEqual;
+import org.joda.time.DateTimeZone;
 import org.junit.Before;
 import org.mockito.Mockito;
 
@@ -71,6 +68,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singleton;
 import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomHistogramGroupConfig;
 import static org.elasticsearch.xpack.core.rollup.RollupField.COUNT_FIELD;
 import static org.hamcrest.Matchers.equalTo;
@@ -82,12 +81,11 @@ public class SearchActionTests extends ESTestCase {
 
     private NamedWriteableRegistry namedWriteableRegistry;
 
-    @Override
     @Before
     public void setUp() throws Exception {
         super.setUp();
-        IndicesModule indicesModule = new IndicesModule(Collections.emptyList());
-        SearchModule searchModule = new SearchModule(Settings.EMPTY, false, Collections.emptyList());
+        IndicesModule indicesModule = new IndicesModule(emptyList());
+        SearchModule searchModule = new SearchModule(Settings.EMPTY, false, emptyList());
         List<NamedWriteableRegistry.Entry> entries = new ArrayList<>();
         entries.addAll(indicesModule.getNamedWriteables());
         entries.addAll(searchModule.getNamedWriteables());
@@ -121,10 +119,9 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testRange() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(groupConfig);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
         Set<RollupJobCaps> caps = new HashSet<>();
         caps.add(cap);
         QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new RangeQueryBuilder("foo").gt(1).timeZone("UTC"), caps);
@@ -133,10 +130,9 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testRangeNullTimeZone() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"), null, null));
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
         Set<RollupJobCaps> caps = new HashSet<>();
         caps.add(cap);
         QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new RangeQueryBuilder("foo").gt(1), caps);
@@ -145,10 +141,9 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testRangeWrongTZ() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"), null, "UTC"));
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
         Set<RollupJobCaps> caps = new HashSet<>();
         caps.add(cap);
         Exception e = expectThrows(IllegalArgumentException.class,
@@ -158,11 +153,10 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testTermQuery() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        final TermsGroupConfig termsConfig = new TermsGroupConfig("foo");
-        final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("date", new DateHistogramInterval("1h")), null, termsConfig);
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final TermsGroupConfig terms = new TermsGroupConfig("foo");
+        final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("boo", new DateHistogramInterval("1h")), null, terms);
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
         Set<RollupJobCaps> caps = new HashSet<>();
         caps.add(cap);
         QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new TermQueryBuilder("foo", "bar"), caps);
@@ -171,16 +165,14 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testTermsQuery() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        final TermsGroupConfig termsConfig = new TermsGroupConfig("foo");
-        final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("date", new DateHistogramInterval("1h")), null, termsConfig);
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final TermsGroupConfig terms = new TermsGroupConfig("foo");
+        final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("boo", new DateHistogramInterval("1h")), null, terms);
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
         Set<RollupJobCaps> caps = new HashSet<>();
         caps.add(cap);
         QueryBuilder original = new TermsQueryBuilder("foo", Arrays.asList("bar", "baz"));
-        QueryBuilder rewritten =
-            TransportRollupSearchAction.rewriteQuery(original, caps);
+        QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(original, caps);
         assertThat(rewritten, instanceOf(TermsQueryBuilder.class));
         assertNotSame(rewritten, original);
         assertThat(((TermsQueryBuilder)rewritten).fieldName(), equalTo("foo.terms.value"));
@@ -188,10 +180,9 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testCompounds() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(groupConfig);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
         Set<RollupJobCaps> caps = new HashSet<>();
         caps.add(cap);
 
@@ -203,10 +194,9 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testMatchAll() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(groupConfig);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
         Set<RollupJobCaps> caps = new HashSet<>();
         caps.add(cap);
         QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new MatchAllQueryBuilder(), caps);
@@ -214,11 +204,10 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testAmbiguousResolution() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        final TermsGroupConfig termsConfig = new TermsGroupConfig("foo");
-        final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")), null, termsConfig);
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
+        final TermsGroupConfig terms = new TermsGroupConfig("foo");
+        final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")), null, terms);
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
         Set<RollupJobCaps> caps = new HashSet<>();
         caps.add(cap);
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
@@ -364,11 +353,10 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testGood() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
         final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(groupConfig);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
-        Set<RollupJobCaps> caps = singletonSet(cap);
+        final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(config);
+        Set<RollupJobCaps> caps = singleton(cap);
 
         String[] normalIndices = new String[]{ESTestCase.randomAlphaOfLength(10)};
         String[] rollupIndices = new String[]{ESTestCase.randomAlphaOfLength(10)};
@@ -381,7 +369,7 @@ public class SearchActionTests extends ESTestCase {
         source.query(getQueryBuilder(1));
         source.size(0);
         source.aggregation(new DateHistogramAggregationBuilder("foo").field("foo")
-                .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()));
+                .dateHistogramInterval(config.getGroupConfig().getDateHistogram().getInterval()));
         SearchRequest request = new SearchRequest(combinedIndices, source);
 
         MultiSearchRequest msearch = TransportRollupSearchAction.createMSearchRequest(request, namedWriteableRegistry, ctx);
@@ -409,10 +397,10 @@ public class SearchActionTests extends ESTestCase {
         source.aggregation(new DateHistogramAggregationBuilder("foo").field("foo").dateHistogramInterval(new DateHistogramInterval("1d")));
         SearchRequest request = new SearchRequest(combinedIndices, source);
 
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo")
-                .setGroupConfig(new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1d"), null, "UTC")))
-                .build();
-        Set<RollupJobCaps> caps = singletonSet(new RollupJobCaps(job));
+        final GroupConfig groupConfig =
+            new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1d"), null, DateTimeZone.UTC.getID()));
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        Set<RollupJobCaps> caps = singleton(new RollupJobCaps(job));
 
         TransportRollupSearchAction.RollupSearchContext ctx
                 = new TransportRollupSearchAction.RollupSearchContext(normalIndices, rollupIndices, caps);
@@ -432,17 +420,15 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testTwoMatchingJobs() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
-
-        RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2").setRollupIndex(job.getRollupIndex());
-        job2.setGroupConfig(group);
+        final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")), null, null);
+        final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
 
         // so that the jobs aren't exactly equal
-        job2.setMetricsConfig(ConfigTestHelpers.randomMetricsConfigs(random()));
-        RollupJobCaps cap2 = new RollupJobCaps(job2.build());
+        final List<MetricConfig> metricConfigs = ConfigTestHelpers.randomMetricsConfigs(random());
+        final RollupJobConfig job2 =
+            new RollupJobConfig("foo2", "index", job.getRollupIndex(), "*/5 * * * * ?", 10,  groupConfig, metricConfigs, null);
+        RollupJobCaps cap2 = new RollupJobCaps(job2);
 
         Set<RollupJobCaps> caps = new HashSet<>(2);
         caps.add(cap);
@@ -479,15 +465,17 @@ public class SearchActionTests extends ESTestCase {
     }
 
     public void testTwoMatchingJobsOneBetter() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
-        job.setGroupConfig(group);
-        RollupJobCaps cap = new RollupJobCaps(job.build());
-
-        RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2").setRollupIndex(job.getRollupIndex());
-        final GroupConfig group2 = new GroupConfig(group.getDateHistogram(), randomHistogramGroupConfig(random()), null);
-        job2.setGroupConfig(group2);
-        RollupJobCaps cap2 = new RollupJobCaps(job2.build());
+        final GroupConfig groupConfig =
+            new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")), null, null);
+        final RollupJobConfig job =
+            new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10,  groupConfig, emptyList(), null);
+        RollupJobCaps cap = new RollupJobCaps(job);
+
+        final GroupConfig groupConfig2 =
+            new GroupConfig(groupConfig.getDateHistogram(), randomHistogramGroupConfig(random()), null);
+        final RollupJobConfig job2 =
+            new RollupJobConfig("foo2", "index", job.getRollupIndex(), "*/5 * * * * ?", 10,  groupConfig2, emptyList(), null);
+        RollupJobCaps cap2 = new RollupJobCaps(job2);
 
         Set<RollupJobCaps> caps = new HashSet<>(2);
         caps.add(cap);
@@ -504,7 +492,7 @@ public class SearchActionTests extends ESTestCase {
         source.query(getQueryBuilder(1));
         source.size(0);
         source.aggregation(new DateHistogramAggregationBuilder("foo").field("foo")
-            .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()));
+                .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()));
         SearchRequest request = new SearchRequest(combinedIndices, source);
 
         MultiSearchRequest msearch = TransportRollupSearchAction.createMSearchRequest(request, namedWriteableRegistry, ctx);
@@ -572,7 +560,7 @@ public class SearchActionTests extends ESTestCase {
         String[] indices = new String[]{"foo"};
 
         String jobName = randomAlphaOfLength(5);
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob(jobName).build();
+        RollupJobConfig job = ConfigTestHelpers.randomRollupJobConfig(random(), jobName);
 
         MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
                 Collections.singletonMap(RollupField.TYPE_NAME,
@@ -616,7 +604,7 @@ public class SearchActionTests extends ESTestCase {
         String[] indices = new String[]{"foo"};
 
         String jobName = randomAlphaOfLength(5);
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob(jobName).build();
+        RollupJobConfig job = ConfigTestHelpers.randomRollupJobConfig(random(), jobName);
 
         MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
                 Collections.singletonMap(RollupField.TYPE_NAME,
@@ -680,7 +668,7 @@ public class SearchActionTests extends ESTestCase {
         String[] indices = new String[]{"foo", "bar"};
 
         String jobName = randomAlphaOfLength(5);
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob(jobName).build();
+        RollupJobConfig job = ConfigTestHelpers.randomRollupJobConfig(random(), jobName);
 
         MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
                 Collections.singletonMap(RollupField.TYPE_NAME,
@@ -715,7 +703,7 @@ public class SearchActionTests extends ESTestCase {
         String[] indices = new String[]{"foo", "bar"};
 
         String jobName = randomAlphaOfLength(5);
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob(jobName).build();
+        RollupJobConfig job = ConfigTestHelpers.randomRollupJobConfig(random(), jobName);
 
         MappingMetaData mappingMeta = new MappingMetaData(RollupField.TYPE_NAME,
                 Collections.singletonMap(RollupField.TYPE_NAME,
@@ -745,7 +733,7 @@ public class SearchActionTests extends ESTestCase {
         SearchResponse protoResponse = mock(SearchResponse.class);
         when(protoResponse.getTook()).thenReturn(new TimeValue(100));
         List<InternalAggregation> protoAggTree = new ArrayList<>(1);
-        InternalAvg internalAvg = new InternalAvg("foo", 10, 2, DocValueFormat.RAW, Collections.emptyList(), null);
+        InternalAvg internalAvg = new InternalAvg("foo", 10, 2, DocValueFormat.RAW, emptyList(), null);
         protoAggTree.add(internalAvg);
         Aggregations protoMockAggs = new InternalAggregations(protoAggTree);
         when(protoResponse.getAggregations()).thenReturn(protoMockAggs);
@@ -785,14 +773,9 @@ public class SearchActionTests extends ESTestCase {
 
         MultiSearchResponse msearchResponse
                 = new MultiSearchResponse(new MultiSearchResponse.Item[]{unrolledResponse, rolledResponse}, 123);
-
-        BigArrays bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService());
-        ScriptService scriptService = mock(ScriptService.class);
-
         SearchResponse response = TransportRollupSearchAction.processResponses(separateIndices, msearchResponse,
                 mock(InternalAggregation.ReduceContext.class));
 
-
         assertNotNull(response);
         Aggregations responseAggs = response.getAggregations();
         assertNotNull(responseAggs);
@@ -800,10 +783,4 @@ public class SearchActionTests extends ESTestCase {
         assertThat(avg.getValue(), IsEqual.equalTo(5.0));
 
     }
-
-    private Set<RollupJobCaps> singletonSet(RollupJobCaps cap) {
-        Set<RollupJobCaps> caps = new HashSet<>();
-        caps.add(cap);
-        return caps;
-    }
 }

+ 4 - 4
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/TransportTaskHelperTests.java

@@ -9,7 +9,6 @@ import org.elasticsearch.tasks.Task;
 import org.elasticsearch.tasks.TaskId;
 import org.elasticsearch.tasks.TaskManager;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
 import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig;
 import org.elasticsearch.xpack.rollup.job.RollupJobTask;
 
@@ -18,6 +17,7 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Consumer;
 
+import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomRollupJobConfig;
 import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -25,7 +25,7 @@ import static org.mockito.Mockito.when;
 public class TransportTaskHelperTests extends ESTestCase {
 
     public void testProcessRequestOneMatching() {
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo").build();
+        RollupJobConfig job = randomRollupJobConfig(random(), "foo");
         TaskManager taskManager = mock(TaskManager.class);
         RollupJobTask task = mock(RollupJobTask.class);
         when(task.getDescription()).thenReturn("rollup_foo");
@@ -58,13 +58,13 @@ public class TransportTaskHelperTests extends ESTestCase {
         Map<Long, Task> tasks = getRandomTasks();
         when(taskManager.getTasks()).thenReturn(tasks);
 
-        RollupJobConfig job = ConfigTestHelpers.getRollupJob("foo").build();
+        RollupJobConfig job = randomRollupJobConfig(random(), "foo");
         RollupJobTask task = mock(RollupJobTask.class);
         when(task.getDescription()).thenReturn("rollup_foo");
         when(task.getConfig()).thenReturn(job);
         tasks.put(1L, task);
 
-        RollupJobConfig job2 = ConfigTestHelpers.getRollupJob("foo").build();
+        RollupJobConfig job2 = randomRollupJobConfig(random(), "foo");
         RollupJobTask task2 = mock(RollupJobTask.class);
         when(task2.getDescription()).thenReturn("rollup_foo");
         when(task2.getConfig()).thenReturn(job2);

+ 2 - 113
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java

@@ -7,13 +7,11 @@ package org.elasticsearch.xpack.rollup.config;
 
 import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
 import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig;
 import org.elasticsearch.xpack.core.rollup.job.GroupConfig;
 import org.elasticsearch.xpack.core.rollup.job.HistogramGroupConfig;
 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.joda.time.DateTimeZone;
 
@@ -23,6 +21,7 @@ import java.util.Map;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomHistogramGroupConfig;
+import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomRollupJobConfig;
 import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomTermsGroupConfig;
 import static org.hamcrest.Matchers.equalTo;
 //TODO split this into dedicated unit test classes (one for each config object)
@@ -55,115 +54,6 @@ public class ConfigTests extends ESTestCase {
         assertThat(e.getMessage(), equalTo("Date histogram must not be null"));
     }
 
-    public void testEmptyGroupAndMetrics() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setGroupConfig(null);
-        job.setMetricsConfig(null);
-
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("At least one grouping or metric must be configured."));
-    }
-
-    public void testEmptyJobID() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob(null);
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("An ID is mandatory."));
-
-        job = ConfigTestHelpers.getRollupJob("");
-        e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("An ID is mandatory."));
-
-        job.setId("");
-        e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("An ID is mandatory."));
-
-        job.setId(null);
-        e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("An ID is mandatory."));
-    }
-
-    public void testEmptyCron() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setCron("");
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("A cron schedule is mandatory."));
-
-        job.setCron(null);
-        e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("A cron schedule is mandatory."));
-    }
-
-    public void testBadCron() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setCron("0 * * *");
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("invalid cron expression [0 * * *]"));
-    }
-
-    public void testEmptyIndexPattern() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setIndexPattern("");
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("An index pattern is mandatory."));
-
-        job.setIndexPattern(null);
-        e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("An index pattern is mandatory."));
-    }
-
-    public void testMatchAllIndexPattern() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setIndexPattern("*");
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("Index pattern must not match all indices (as it would match it's own rollup index"));
-    }
-
-    public void testMatchOwnRollupPatternPrefix() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setIndexPattern("foo-*");
-        job.setRollupIndex("foo-rollup");
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("Index pattern would match rollup index name which is not allowed."));
-    }
-
-    public void testMatchOwnRollupPatternSuffix() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setIndexPattern("*-rollup");
-        job.setRollupIndex("foo-rollup");
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("Index pattern would match rollup index name which is not allowed."));
-    }
-
-    public void testIndexPatternIdenticalToRollup() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setIndexPattern("foo");
-        job.setRollupIndex("foo");
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("Rollup index may not be the same as the index pattern."));
-    }
-
-    public void testEmptyRollupIndex() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setRollupIndex("");
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("A rollup index name is mandatory."));
-
-        job.setRollupIndex(null);
-        e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("A rollup index name is mandatory."));
-    }
-
-    public void testBadSize() {
-        RollupJobConfig.Builder job = ConfigTestHelpers.getRollupJob("foo");
-        job.setPageSize(-1);
-        Exception e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("Parameter [page_size] is mandatory and  must be a positive long."));
-
-        job.setPageSize(0);
-        e = expectThrows(IllegalArgumentException.class, job::build);
-        assertThat(e.getMessage(), equalTo("Parameter [page_size] is mandatory and  must be a positive long."));
-    }
-
     public void testEmptyDateHistoField() {
         Exception e = expectThrows(IllegalArgumentException.class,
             () -> new DateHistogramGroupConfig(null, DateHistogramInterval.HOUR));
@@ -225,8 +115,7 @@ public class ConfigTests extends ESTestCase {
         Map<String, String> headers = new HashMap<>(1);
         headers.put("es-security-runas-user", "foo");
         headers.put("_xpack_security_authentication", "bar");
-        RollupJobConfig config = ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build();
-        RollupJob job = new RollupJob(config, headers);
+        RollupJob job = new RollupJob(randomRollupJobConfig(random()), headers);
         String json = job.toString();
         assertFalse(json.contains("authentication"));
         assertFalse(json.contains("security"));

+ 2 - 9
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java

@@ -434,15 +434,8 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
     }
 
     private RollupJobConfig createJob(String rollupIndex, GroupConfig groupConfig, List<MetricConfig> metricConfigs) {
-        return new RollupJobConfig.Builder()
-                .setId(randomAlphaOfLength(10))
-                .setIndexPattern(randomAlphaOfLength(10))
-                .setRollupIndex(rollupIndex)
-                .setGroupConfig(groupConfig)
-                .setMetricsConfig(metricConfigs)
-                .setCron(ConfigTestHelpers.getCronString())
-                .setPageSize(randomIntBetween(1, 100))
-                .build();
+        return new RollupJobConfig(randomAlphaOfLength(10), randomAlphaOfLength(10), rollupIndex, ConfigTestHelpers.randomCron(),
+            randomIntBetween(1, 100), groupConfig, metricConfigs, ConfigTestHelpers.randomTimeout(random()));
     }
 
     static Map<String, Object> asMap(Object... fields) {

+ 12 - 24
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerStateTests.java

@@ -216,8 +216,7 @@ public class RollupIndexerStateTests extends ESTestCase {
     }
 
     public void testStarted() throws Exception {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         final ExecutorService executor = Executors.newFixedThreadPool(1);
         try {
@@ -236,8 +235,7 @@ public class RollupIndexerStateTests extends ESTestCase {
     }
 
     public void testIndexing() throws Exception {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         final ExecutorService executor = Executors.newFixedThreadPool(1);
         try {
@@ -304,8 +302,7 @@ public class RollupIndexerStateTests extends ESTestCase {
 
     public void testAbortDuringSearch() throws Exception {
         final AtomicBoolean aborted = new AtomicBoolean(false);
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         final ExecutorService executor = Executors.newFixedThreadPool(1);
         final CountDownLatch latch = new CountDownLatch(1);
@@ -349,8 +346,7 @@ public class RollupIndexerStateTests extends ESTestCase {
 
     public void testAbortAfterCompletion() throws Exception {
         final AtomicBoolean aborted = new AtomicBoolean(false);
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         final ExecutorService executor = Executors.newFixedThreadPool(1);
 
@@ -435,8 +431,7 @@ public class RollupIndexerStateTests extends ESTestCase {
     }
 
     public void testStopIndexing() throws Exception {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         final ExecutorService executor = Executors.newFixedThreadPool(1);
         try {
@@ -458,8 +453,7 @@ public class RollupIndexerStateTests extends ESTestCase {
     }
 
     public void testAbortIndexing() throws Exception {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         final ExecutorService executor = Executors.newFixedThreadPool(1);
         try {
@@ -486,8 +480,7 @@ public class RollupIndexerStateTests extends ESTestCase {
     }
 
     public void testAbortStarted() throws Exception {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         final ExecutorService executor = Executors.newFixedThreadPool(1);
         try {
@@ -513,8 +506,7 @@ public class RollupIndexerStateTests extends ESTestCase {
     }
 
     public void testMultipleJobTriggering() throws Exception {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         final ExecutorService executor = Executors.newFixedThreadPool(1);
         try {
@@ -554,8 +546,7 @@ public class RollupIndexerStateTests extends ESTestCase {
     // deal with it everyhwere
     public void testUnknownKey() throws Exception {
         AtomicBoolean isFinished = new AtomicBoolean(false);
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         Function<SearchRequest, SearchResponse> searchFunction = searchRequest -> {
             Aggregations aggs = new Aggregations(Collections.singletonList(new CompositeAggregation() {
@@ -659,8 +650,7 @@ public class RollupIndexerStateTests extends ESTestCase {
     public void testFailureWhileStopping() throws Exception {
         AtomicBoolean isFinished = new AtomicBoolean(false);
 
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         Function<SearchRequest, SearchResponse> searchFunction = searchRequest -> {
             Aggregations aggs = new Aggregations(Collections.singletonList(new CompositeAggregation() {
@@ -762,8 +752,7 @@ public class RollupIndexerStateTests extends ESTestCase {
 
     public void testSearchShardFailure() throws Exception {
         AtomicBoolean isFinished = new AtomicBoolean(false);
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         Function<SearchRequest, SearchResponse> searchFunction = searchRequest -> {
             ShardSearchFailure[] failures = new ShardSearchFailure[]{new ShardSearchFailure(new RuntimeException("failed"))};
@@ -806,8 +795,7 @@ public class RollupIndexerStateTests extends ESTestCase {
 
     public void testBulkFailure() throws Exception {
         AtomicBoolean isFinished = new AtomicBoolean(false);
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(ESTestCase.randomAlphaOfLengthBetween(1, 10)).build(),
-            Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         AtomicReference<IndexerState> state = new AtomicReference<>(IndexerState.STOPPED);
         Function<SearchRequest, SearchResponse> searchFunction = searchRequest -> {
             Aggregations aggs = new Aggregations(Collections.singletonList(new CompositeAggregation() {

+ 18 - 18
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupJobTaskTests.java

@@ -58,7 +58,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testInitialStatusStopped() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.STOPPED, Collections.singletonMap("foo", "bar"), randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -71,7 +71,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testInitialStatusAborting() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.ABORTING, Collections.singletonMap("foo", "bar"), randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -84,7 +84,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testInitialStatusStopping() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.STOPPING, Collections.singletonMap("foo", "bar"), randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -97,7 +97,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testInitialStatusStarted() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.STARTED, Collections.singletonMap("foo", "bar"), randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -110,7 +110,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testInitialStatusIndexingOldID() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.INDEXING, Collections.singletonMap("foo", "bar"), false);
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -124,7 +124,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testInitialStatusIndexingNewID() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.INDEXING, Collections.singletonMap("foo", "bar"), true);
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -138,7 +138,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testNoInitialStatus() {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
         SchedulerEngine schedulerEngine = new SchedulerEngine(Clock.systemUTC());
@@ -150,7 +150,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testStartWhenStarted() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.STARTED, Collections.singletonMap("foo", "bar"), randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -179,7 +179,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testStartWhenStopping() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
         when(client.threadPool()).thenReturn(pool);
@@ -258,7 +258,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testStartWhenStopped() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.STOPPED, Collections.singletonMap("foo", "bar"), randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -296,7 +296,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testTriggerUnrelated() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.STOPPED, Collections.singletonMap("foo", "bar"), randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -337,7 +337,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testTrigger() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
         when(client.threadPool()).thenReturn(pool);
@@ -380,7 +380,7 @@ public class RollupJobTaskTests extends ESTestCase {
     @SuppressWarnings("unchecked")
     public void testTriggerWithoutHeaders() throws InterruptedException {
         final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
 
@@ -465,7 +465,7 @@ public class RollupJobTaskTests extends ESTestCase {
         Map<String, String> headers = new HashMap<>(1);
         headers.put("es-security-runas-user", "foo");
         headers.put("_xpack_security_authentication", "bar");
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), headers);
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), headers);
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
 
@@ -553,7 +553,7 @@ public class RollupJobTaskTests extends ESTestCase {
         Map<String, String> headers = new HashMap<>(1);
         headers.put("es-security-runas-user", "foo");
         headers.put("_xpack_security_authentication", "bar");
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), headers);
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), headers);
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
 
@@ -637,7 +637,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testStopWhenStopped() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.STOPPED, null, randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
@@ -663,7 +663,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testStopWhenStopping() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);
         when(client.threadPool()).thenReturn(pool);
@@ -744,7 +744,7 @@ public class RollupJobTaskTests extends ESTestCase {
     }
 
     public void testStopWhenAborting() throws InterruptedException {
-        RollupJob job = new RollupJob(ConfigTestHelpers.getRollupJob(randomAlphaOfLength(5)).build(), Collections.emptyMap());
+        RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap());
         RollupJobStatus status = new RollupJobStatus(IndexerState.STOPPED, null, randomBoolean());
         Client client = mock(Client.class);
         when(client.settings()).thenReturn(Settings.EMPTY);