From 08e4f4be42566c24370c7bedf8ed8d3611f29939 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 2 Aug 2018 17:55:00 +0200 Subject: [PATCH] [Rollup] Remove builders from HistoGroupConfig (#32533) Related to #29827 --- .../xpack/core/rollup/job/GroupConfig.java | 22 ++-- ...pConfig.java => HistogramGroupConfig.java} | 101 +++++++----------- .../xpack/core/rollup/ConfigTestHelpers.java | 20 ++-- ...HistogramGroupConfigSerializingTests.java} | 42 +++----- .../rollup/RollupJobIdentifierUtilTests.java | 19 +--- .../rollup/action/SearchActionTests.java | 3 +- .../xpack/rollup/config/ConfigTests.java | 25 ++--- .../xpack/rollup/job/IndexerUtilsTests.java | 8 +- 8 files changed, 97 insertions(+), 143 deletions(-) rename x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/{HistoGroupConfig.java => HistogramGroupConfig.java} (67%) rename x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/{HistoGroupConfigSerializingTests.java => HistogramGroupConfigSerializingTests.java} (74%) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java index f6977394f4af2..a450c0bfab763 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java @@ -44,18 +44,18 @@ public class GroupConfig implements Writeable, ToXContentObject { private static final ParseField TERMS = new ParseField("terms"); private final DateHistoGroupConfig dateHisto; - private final HistoGroupConfig histo; + private final HistogramGroupConfig histo; private final TermsGroupConfig terms; public static final ObjectParser PARSER = new ObjectParser<>(NAME, GroupConfig.Builder::new); static { PARSER.declareObject(GroupConfig.Builder::setDateHisto, (p,c) -> DateHistoGroupConfig.PARSER.apply(p,c).build(), DATE_HISTO); - PARSER.declareObject(GroupConfig.Builder::setHisto, (p,c) -> HistoGroupConfig.PARSER.apply(p,c).build(), HISTO); + PARSER.declareObject(GroupConfig.Builder::setHisto, (p,c) -> HistogramGroupConfig.fromXContent(p), HISTO); PARSER.declareObject(GroupConfig.Builder::setTerms, (p,c) -> TermsGroupConfig.fromXContent(p), TERMS); } - private GroupConfig(DateHistoGroupConfig dateHisto, @Nullable HistoGroupConfig histo, @Nullable TermsGroupConfig terms) { + private GroupConfig(DateHistoGroupConfig dateHisto, @Nullable HistogramGroupConfig histo, @Nullable TermsGroupConfig terms) { this.dateHisto = Objects.requireNonNull(dateHisto, "A date_histogram group is mandatory"); this.histo = histo; this.terms = terms; @@ -63,7 +63,7 @@ private GroupConfig(DateHistoGroupConfig dateHisto, @Nullable HistoGroupConfig h GroupConfig(StreamInput in) throws IOException { dateHisto = new DateHistoGroupConfig(in); - histo = in.readOptionalWriteable(HistoGroupConfig::new); + histo = in.readOptionalWriteable(HistogramGroupConfig::new); terms = in.readOptionalWriteable(TermsGroupConfig::new); } @@ -71,7 +71,7 @@ public DateHistoGroupConfig getDateHisto() { return dateHisto; } - public HistoGroupConfig getHisto() { + public HistogramGroupConfig getHisto() { return histo; } @@ -83,7 +83,7 @@ public Set getAllFields() { Set fields = new HashSet<>(); fields.add(dateHisto.getField()); if (histo != null) { - fields.addAll(histo.getAllFields()); + fields.addAll(asList(histo.getFields())); } if (terms != null) { fields.addAll(asList(terms.getFields())); @@ -109,9 +109,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws dateHisto.toXContent(builder, params); builder.endObject(); if (histo != null) { - builder.startObject(HISTO.getPreferredName()); - histo.toXContent(builder, params); - builder.endObject(); + builder.field(HISTO.getPreferredName(), histo); } if (terms != null) { builder.field(TERMS.getPreferredName(), terms); @@ -156,7 +154,7 @@ public String toString() { public static class Builder { private DateHistoGroupConfig dateHisto; - private HistoGroupConfig histo; + private HistogramGroupConfig histo; private TermsGroupConfig terms; public DateHistoGroupConfig getDateHisto() { @@ -168,11 +166,11 @@ public GroupConfig.Builder setDateHisto(DateHistoGroupConfig dateHisto) { return this; } - public HistoGroupConfig getHisto() { + public HistogramGroupConfig getHisto() { return histo; } - public GroupConfig.Builder setHisto(HistoGroupConfig histo) { + public GroupConfig.Builder setHisto(HistogramGroupConfig histo) { this.histo = histo; return this; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistoGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfig.java similarity index 67% rename from x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistoGroupConfig.java rename to x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfig.java index 87de9e165345e..4f67978e4bc92 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistoGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfig.java @@ -12,9 +12,10 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ObjectParser; -import org.elasticsearch.common.xcontent.ToXContentFragment; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.aggregations.bucket.composite.CompositeValuesSourceBuilder; import org.elasticsearch.search.aggregations.bucket.composite.HistogramValuesSourceBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder; @@ -27,9 +28,10 @@ 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; + /** * The configuration object for the histograms in the rollup config * @@ -42,28 +44,36 @@ * ] * } */ -public class HistoGroupConfig implements Writeable, ToXContentFragment { - private static final String NAME = "histo_group_config"; - public static final ObjectParser PARSER - = new ObjectParser<>(NAME, HistoGroupConfig.Builder::new); +public class HistogramGroupConfig implements Writeable, ToXContentObject { - private static final ParseField INTERVAL = new ParseField("interval"); - private static final ParseField FIELDS = new ParseField("fields"); + public static final String NAME = "histogram"; + private static final String INTERVAL = "interval"; + private static final String FIELDS = "fields"; + private static final ConstructingObjectParser PARSER; + static { + PARSER = new ConstructingObjectParser<>(NAME, args -> { + @SuppressWarnings("unchecked") List fields = (List) args[1]; + return new HistogramGroupConfig((long) args[0], fields != null ? fields.toArray(new String[fields.size()]) : null); + }); + PARSER.declareLong(constructorArg(), new ParseField(INTERVAL)); + PARSER.declareStringArray(constructorArg(), new ParseField(FIELDS)); + } private final long interval; private final String[] fields; - static { - PARSER.declareLong(HistoGroupConfig.Builder::setInterval, INTERVAL); - PARSER.declareStringArray(HistoGroupConfig.Builder::setFields, FIELDS); - } - - private HistoGroupConfig(long interval, String[] fields) { + public HistogramGroupConfig(final long interval, final String... fields) { + if (interval <= 0) { + throw new IllegalArgumentException("Interval must be a positive long"); + } + if (fields == null || fields.length == 0) { + throw new IllegalArgumentException("Fields must have at least one value"); + } this.interval = interval; this.fields = fields; } - HistoGroupConfig(StreamInput in) throws IOException { + HistogramGroupConfig(final StreamInput in) throws IOException { interval = in.readVLong(); fields = in.readStringArray(); } @@ -101,7 +111,7 @@ public List> toBuilders() { public Map toAggCap() { Map map = new HashMap<>(2); map.put("agg", HistogramAggregationBuilder.NAME); - map.put(INTERVAL.getPreferredName(), interval); + map.put(INTERVAL, interval); return map; } @@ -109,10 +119,6 @@ public Map getMetadata() { return Collections.singletonMap(RollupField.formatMetaField(RollupField.INTERVAL), interval); } - public Set getAllFields() { - return Arrays.stream(fields).collect(Collectors.toSet()); - } - public void validateMappings(Map> fieldCapsResponse, ActionRequestValidationException validationException) { @@ -138,9 +144,13 @@ public void validateMappings(Map> fieldCa } @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(INTERVAL.getPreferredName(), interval); - builder.field(FIELDS.getPreferredName(), fields); + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + builder.startObject(); + { + builder.field(INTERVAL, interval); + builder.field(FIELDS, fields); + } + builder.endObject(); return builder; } @@ -151,19 +161,15 @@ public void writeTo(StreamOutput out) throws IOException { } @Override - public boolean equals(Object other) { + public boolean equals(final Object other) { if (this == other) { return true; } - if (other == null || getClass() != other.getClass()) { return false; } - - HistoGroupConfig that = (HistoGroupConfig) other; - - return Objects.equals(this.interval, that.interval) - && Arrays.equals(this.fields, that.fields); + final HistogramGroupConfig that = (HistogramGroupConfig) other; + return Objects.equals(interval, that.interval) && Arrays.equals(fields, that.fields); } @Override @@ -176,36 +182,7 @@ public String toString() { return Strings.toString(this, true, true); } - public static class Builder { - private long interval = 0; - private List fields; - - public long getInterval() { - return interval; - } - - public HistoGroupConfig.Builder setInterval(long interval) { - this.interval = interval; - return this; - } - - public List getFields() { - return fields; - } - - public HistoGroupConfig.Builder setFields(List fields) { - this.fields = fields; - return this; - } - - public HistoGroupConfig build() { - if (interval <= 0) { - throw new IllegalArgumentException("Parameter [" + INTERVAL.getPreferredName() + "] must be a positive long."); - } - if (fields == null || fields.isEmpty()) { - throw new IllegalArgumentException("Parameter [" + FIELDS + "] must have at least one value."); - } - return new HistoGroupConfig(interval, fields.toArray(new String[0])); - } + public static HistogramGroupConfig fromXContent(final XContentParser parser) throws IOException { + return PARSER.parse(parser, null); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java index b2e2730760f9c..36b15a1e1f5da 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java @@ -5,12 +5,13 @@ */ package org.elasticsearch.xpack.core.rollup; +import com.carrotsearch.randomizedtesting.generators.RandomNumbers; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.rollup.job.DateHistoGroupConfig; import org.elasticsearch.xpack.core.rollup.job.GroupConfig; -import org.elasticsearch.xpack.core.rollup.job.HistoGroupConfig; +import org.elasticsearch.xpack.core.rollup.job.HistogramGroupConfig; import org.elasticsearch.xpack.core.rollup.job.MetricConfig; import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; @@ -50,7 +51,7 @@ public static GroupConfig.Builder getGroupConfig() { GroupConfig.Builder groupBuilder = new GroupConfig.Builder(); groupBuilder.setDateHisto(getDateHisto().build()); if (ESTestCase.randomBoolean()) { - groupBuilder.setHisto(getHisto().build()); + groupBuilder.setHisto(randomHistogramGroupConfig(ESTestCase.random())); } if (ESTestCase.randomBoolean()) { groupBuilder.setTerms(randomTermsGroupConfig(ESTestCase.random())); @@ -102,13 +103,6 @@ public static DateHistoGroupConfig.Builder getDateHisto() { return dateHistoBuilder; } - public static HistoGroupConfig.Builder getHisto() { - HistoGroupConfig.Builder histoBuilder = new HistoGroupConfig.Builder(); - histoBuilder.setInterval(ESTestCase.randomIntBetween(1,10000)); - histoBuilder.setFields(getFields()); - return histoBuilder; - } - public static List getFields() { return IntStream.range(0, ESTestCase.randomIntBetween(1, 10)) .mapToObj(n -> ESTestCase.randomAlphaOfLengthBetween(5, 10)) @@ -125,6 +119,10 @@ public static String getCronString() { " " + (ESTestCase.randomBoolean() ? "*" : String.valueOf(ESTestCase.randomIntBetween(1970, 2199))); //year } + public static HistogramGroupConfig randomHistogramGroupConfig(final Random random) { + return new HistogramGroupConfig(randomInterval(random), randomFields(random)); + } + public static TermsGroupConfig randomTermsGroupConfig(final Random random) { return new TermsGroupConfig(randomFields(random)); } @@ -141,4 +139,8 @@ private static String[] randomFields(final Random random) { private static String randomField(final Random random) { return randomAsciiAlphanumOfLengthBetween(random, 5, 10); } + + private static long randomInterval(final Random random) { + return RandomNumbers.randomLongBetween(random, 1L, Long.MAX_VALUE); + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/HistoGroupConfigSerializingTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfigSerializingTests.java similarity index 74% rename from x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/HistoGroupConfigSerializingTests.java rename to x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfigSerializingTests.java index 92e7d8b9643e6..ef81b235b1fba 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/HistoGroupConfigSerializingTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfigSerializingTests.java @@ -10,7 +10,6 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers; import org.elasticsearch.xpack.core.rollup.RollupField; import java.io.IOException; @@ -18,34 +17,33 @@ import java.util.HashMap; import java.util.Map; +import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomHistogramGroupConfig; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class HistoGroupConfigSerializingTests extends AbstractSerializingTestCase { +public class HistogramGroupConfigSerializingTests extends AbstractSerializingTestCase { + @Override - protected HistoGroupConfig doParseInstance(XContentParser parser) throws IOException { - return HistoGroupConfig.PARSER.apply(parser, null).build(); + protected HistogramGroupConfig doParseInstance(XContentParser parser) throws IOException { + return HistogramGroupConfig.fromXContent(parser); } @Override - protected Writeable.Reader instanceReader() { - return HistoGroupConfig::new; + protected Writeable.Reader instanceReader() { + return HistogramGroupConfig::new; } @Override - protected HistoGroupConfig createTestInstance() { - return ConfigTestHelpers.getHisto().build(); + protected HistogramGroupConfig createTestInstance() { + return randomHistogramGroupConfig(random()); } public void testValidateNoMapping() throws IOException { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); - HistoGroupConfig config = new HistoGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .setInterval(123) - .build(); + HistogramGroupConfig config = new HistogramGroupConfig(132, "my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] field with name [my_field] in any of the " + "indices matching the index pattern.")); @@ -60,10 +58,7 @@ public void testValidateNomatchingField() throws IOException { FieldCapabilities fieldCaps = mock(FieldCapabilities.class); responseMap.put("some_other_field", Collections.singletonMap("long", fieldCaps)); - HistoGroupConfig config = new HistoGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .setInterval(123) - .build(); + HistogramGroupConfig config = new HistogramGroupConfig(132, "my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] field with name [my_field] in any of the " + "indices matching the index pattern.")); @@ -78,10 +73,7 @@ public void testValidateFieldWrongType() throws IOException { FieldCapabilities fieldCaps = mock(FieldCapabilities.class); responseMap.put("my_field", Collections.singletonMap("keyword", fieldCaps)); - HistoGroupConfig config = new HistoGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .setInterval(123) - .build(); + HistogramGroupConfig config = new HistogramGroupConfig(132, "my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("The field referenced by a histo group must be a [numeric] type, but " + "found [keyword] for field [my_field]")); @@ -97,10 +89,7 @@ public void testValidateFieldMatchingNotAggregatable() throws IOException { when(fieldCaps.isAggregatable()).thenReturn(false); responseMap.put("my_field", Collections.singletonMap("long", fieldCaps)); - HistoGroupConfig config = new HistoGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .setInterval(123) - .build(); + HistogramGroupConfig config = new HistogramGroupConfig(132, "my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable across all indices, but is not.")); } @@ -115,10 +104,7 @@ public void testValidateMatchingField() throws IOException { String mappingType = randomFrom(RollupField.NUMERIC_FIELD_MAPPER_TYPES); responseMap.put("my_field", Collections.singletonMap(mappingType, fieldCaps)); - HistoGroupConfig config = new HistoGroupConfig.Builder() - .setFields(Collections.singletonList("my_field")) - .setInterval(123) - .build(); + HistogramGroupConfig config = new HistogramGroupConfig(132, "my_field"); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); } diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java index 376ba00b063d8..f1ef953c011a1 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java @@ -19,7 +19,7 @@ import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps; import org.elasticsearch.xpack.core.rollup.job.DateHistoGroupConfig; import org.elasticsearch.xpack.core.rollup.job.GroupConfig; -import org.elasticsearch.xpack.core.rollup.job.HistoGroupConfig; +import org.elasticsearch.xpack.core.rollup.job.HistogramGroupConfig; import org.elasticsearch.xpack.core.rollup.job.MetricConfig; import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; @@ -267,7 +267,7 @@ public void testComparableNoHistoVsHisto() { RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2").setRollupIndex(job.getRollupIndex()); GroupConfig.Builder group2 = ConfigTestHelpers.getGroupConfig(); group2.setDateHisto(new DateHistoGroupConfig.Builder().setField("foo").setInterval(new DateHistogramInterval("1h")).build()) - .setHisto(ConfigTestHelpers.getHisto().setInterval(100).setFields(Collections.singletonList("bar")).build()) + .setHisto(new HistogramGroupConfig(100L, "bar")) .setTerms(null); job2.setGroupConfig(group2.build()); RollupJobCaps cap2 = new RollupJobCaps(job2.build()); @@ -329,10 +329,7 @@ public void testHistoSameNameWrongTypeInCaps() { .setField("foo") // <-- NOTE same name but wrong type .setTimeZone(DateTimeZone.UTC) .build()) - .setHisto(new HistoGroupConfig.Builder() - .setFields(Collections.singletonList("baz")) // <-- NOTE right type but wrong name - .setInterval(1L) - .build()) + .setHisto(new HistogramGroupConfig(1L, "baz")) // <-- NOTE right type but wrong name .build()) .setMetricsConfig(Arrays.asList(new MetricConfig.Builder() .setField("max_field") @@ -443,10 +440,7 @@ public void testHistoMissingFieldInCaps() { .setField("bar") .setTimeZone(DateTimeZone.UTC) .build()) - .setHisto(new HistoGroupConfig.Builder() - .setFields(Collections.singletonList("baz")) // <-- NOTE note different field from one used in query - .setInterval(1L) - .build()) + .setHisto(new HistogramGroupConfig(1L, "baz")) // <-- NOTE right type but wrong name .build()) .setMetricsConfig(Arrays.asList(new MetricConfig.Builder() .setField("max_field") @@ -476,10 +470,7 @@ public void testNoMatchingHistoInterval() { .setField("foo") .setTimeZone(DateTimeZone.UTC) .build()) - .setHisto(new HistoGroupConfig.Builder() - .setFields(Collections.singletonList("bar")) - .setInterval(100L) // <--- interval in job is much higher than agg interval above - .build()) + .setHisto(new HistogramGroupConfig(1L, "baz")) // <-- NOTE right type but wrong name .build()) .build(); Set caps = singletonSet(new RollupJobCaps(job)); diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java index c2eec92c41080..5b2f22dcd2db2 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java @@ -71,6 +71,7 @@ import java.util.Map; import java.util.Set; +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; import static org.hamcrest.Matchers.instanceOf; @@ -502,7 +503,7 @@ public void testTwoMatchingJobsOneBetter() { RollupJobConfig.Builder job2 = ConfigTestHelpers.getRollupJob("foo2").setRollupIndex(job.getRollupIndex()); GroupConfig.Builder group2 = ConfigTestHelpers.getGroupConfig(); group2.setDateHisto(group.getDateHisto()) - .setHisto(ConfigTestHelpers.getHisto().build()) + .setHisto(randomHistogramGroupConfig(random())) .setTerms(null); job2.setGroupConfig(group2.build()); RollupJobCaps cap2 = new RollupJobCaps(job2.build()); diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index d037bb72e6408..d5fd300b68747 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -9,7 +9,7 @@ import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers; import org.elasticsearch.xpack.core.rollup.job.DateHistoGroupConfig; import org.elasticsearch.xpack.core.rollup.job.GroupConfig; -import org.elasticsearch.xpack.core.rollup.job.HistoGroupConfig; +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; @@ -60,7 +60,7 @@ public void testEmptyGroup() { public void testNoDateHisto() { GroupConfig.Builder groupConfig = new GroupConfig.Builder(); groupConfig.setTerms(ConfigTestHelpers.randomTermsGroupConfig(random())); - groupConfig.setHisto(ConfigTestHelpers.getHisto().build()); + groupConfig.setHisto(ConfigTestHelpers.randomHistogramGroupConfig(random())); Exception e = expectThrows(IllegalArgumentException.class, groupConfig::build); assertThat(e.getMessage(), equalTo("A date_histogram group is mandatory")); @@ -202,24 +202,19 @@ public void testNullTimeZone() { } public void testEmptyHistoField() { - HistoGroupConfig.Builder config = ConfigTestHelpers.getHisto(); - config.setFields(null); - Exception e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [fields] must have at least one value.")); + Exception e = expectThrows(IllegalArgumentException.class, () -> new HistogramGroupConfig(1L, (String[]) null)); + assertThat(e.getMessage(), equalTo("Fields must have at least one value")); - config.setFields(Collections.emptyList()); - e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [fields] must have at least one value.")); + e = expectThrows(IllegalArgumentException.class, () -> new HistogramGroupConfig(1L, new String[0])); + assertThat(e.getMessage(), equalTo("Fields must have at least one value")); } public void testBadHistoIntervals() { - HistoGroupConfig.Builder config = new HistoGroupConfig.Builder().setFields(Collections.singletonList("foo")); - Exception e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [interval] must be a positive long.")); + Exception e = expectThrows(IllegalArgumentException.class, () -> new HistogramGroupConfig(0L, "foo", "bar")); + assertThat(e.getMessage(), equalTo("Interval must be a positive long")); - config.setInterval(-1); - e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [interval] must be a positive long.")); + e = expectThrows(IllegalArgumentException.class, () -> new HistogramGroupConfig(-1L, "foo", "bar")); + assertThat(e.getMessage(), equalTo("Interval must be a positive long")); } public void testEmptyTermsField() { diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java index 29cda6ece7503..f36ef3b00c70d 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java @@ -38,6 +38,7 @@ import org.elasticsearch.xpack.core.rollup.RollupField; import org.elasticsearch.xpack.core.rollup.job.DateHistoGroupConfig; 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.RollupJobStats; import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; @@ -52,6 +53,7 @@ import java.util.List; import java.util.Map; +import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomHistogramGroupConfig; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -353,8 +355,10 @@ public void testKeyOrdering() { return foos; }); + // The content of the config don't actually matter for this test + // because the test is just looking at agg keys GroupConfig.Builder groupConfig = ConfigTestHelpers.getGroupConfig(); - groupConfig.setHisto(ConfigTestHelpers.getHisto().setFields(Collections.singletonList("abc")).build()); + groupConfig.setHisto(new HistogramGroupConfig(123L, "abc")); List docs = IndexerUtils.processBuckets(composite, "foo", new RollupJobStats(), groupConfig.build(), "foo"); assertThat(docs.size(), equalTo(1)); @@ -383,7 +387,7 @@ public void testNullKeys() { }); GroupConfig.Builder groupConfig = ConfigTestHelpers.getGroupConfig(); - groupConfig.setHisto(ConfigTestHelpers.getHisto().setFields(Collections.singletonList("abc")).build()); + groupConfig.setHisto(randomHistogramGroupConfig(random())); List docs = IndexerUtils.processBuckets(composite, "foo", new RollupJobStats(), groupConfig.build(), "foo"); assertThat(docs.size(), equalTo(1));