diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java index 006d8c35c324d..1843db5b3c304 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.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.metrics.avg.AvgAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.max.MaxAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.min.MinAggregationBuilder; @@ -32,6 +33,8 @@ import java.util.Objects; import java.util.stream.Collectors; +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; + /** * The configuration object for the metrics portion of a rollup job config * @@ -48,14 +51,7 @@ * ] * } */ -public class MetricConfig implements Writeable, ToXContentFragment { - private static final String NAME = "metric_config"; - - private String field; - private List metrics; - - private static final ParseField FIELD = new ParseField("field"); - private static final ParseField METRICS = new ParseField("metrics"); +public class MetricConfig implements Writeable, ToXContentObject { // TODO: replace these with an enum private static final ParseField MIN = new ParseField("min"); @@ -64,27 +60,54 @@ public class MetricConfig implements Writeable, ToXContentFragment { private static final ParseField AVG = new ParseField("avg"); private static final ParseField VALUE_COUNT = new ParseField("value_count"); - public static final ObjectParser PARSER = new ObjectParser<>(NAME, MetricConfig.Builder::new); - + private static final String NAME = "metrics"; + private static final String FIELD = "field"; + private static final String METRICS = "metrics"; + private static final ConstructingObjectParser PARSER; static { - PARSER.declareString(MetricConfig.Builder::setField, FIELD); - PARSER.declareStringArray(MetricConfig.Builder::setMetrics, METRICS); + PARSER = new ConstructingObjectParser<>(NAME, args -> { + @SuppressWarnings("unchecked") List metrics = (List) args[1]; + return new MetricConfig((String) args[0], metrics); + }); + PARSER.declareString(constructorArg(), new ParseField(FIELD)); + PARSER.declareStringArray(constructorArg(), new ParseField(METRICS)); } - MetricConfig(String name, List metrics) { - this.field = name; + private final String field; + private final List metrics; + + public MetricConfig(final String field, final List metrics) { + if (field == null || field.isEmpty()) { + throw new IllegalArgumentException("Field must be a non-null, non-empty string"); + } + if (metrics == null || metrics.isEmpty()) { + throw new IllegalArgumentException("Metrics must be a non-null, non-empty array of strings"); + } + metrics.forEach(m -> { + if (RollupField.SUPPORTED_METRICS.contains(m) == false) { + throw new IllegalArgumentException("Unsupported metric [" + m + "]. " + + "Supported metrics include: " + RollupField.SUPPORTED_METRICS); + } + }); + this.field = field; this.metrics = metrics; } - MetricConfig(StreamInput in) throws IOException { + MetricConfig(final StreamInput in) throws IOException { field = in.readString(); metrics = in.readList(StreamInput::readString); } + /** + * @return the name of the field used in the metric configuration. Never {@code null}. + */ public String getField() { return field; } + /** + * @return the names of the metrics used in the metric configuration. Never {@code null}. + */ public List getMetrics() { return metrics; } @@ -159,10 +182,13 @@ public void validateMappings(Map> fieldCa } @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(FIELD.getPreferredName(), field); - builder.field(METRICS.getPreferredName(), metrics); - return builder; + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + builder.startObject(); + { + builder.field(FIELD, field); + builder.field(METRICS, metrics); + } + return builder.endObject(); } @Override @@ -172,19 +198,16 @@ 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; } - MetricConfig that = (MetricConfig) other; - - return Objects.equals(this.field, that.field) - && Objects.equals(this.metrics, that.metrics); + final MetricConfig that = (MetricConfig) other; + return Objects.equals(field, that.field) && Objects.equals(metrics, that.metrics); } @Override @@ -197,52 +220,7 @@ public String toString() { return Strings.toString(this, true, true); } - - public static class Builder { - private String field; - private List metrics; - - public Builder() { - } - - public Builder(MetricConfig config) { - this.field = config.getField(); - this.metrics = config.getMetrics(); - } - - public String getField() { - return field; - } - - public MetricConfig.Builder setField(String field) { - this.field = field; - return this; - } - - public List getMetrics() { - return metrics; - } - - public MetricConfig.Builder setMetrics(List metrics) { - this.metrics = metrics; - return this; - } - - public MetricConfig build() { - if (Strings.isNullOrEmpty(field) == true) { - throw new IllegalArgumentException("Parameter [" + FIELD.getPreferredName() + "] must be a non-null, non-empty string."); - } - if (metrics == null || metrics.isEmpty()) { - throw new IllegalArgumentException("Parameter [" + METRICS.getPreferredName() - + "] must be a non-null, non-empty array of strings."); - } - metrics.forEach(m -> { - if (RollupField.SUPPORTED_METRICS.contains(m) == false) { - throw new IllegalArgumentException("Unsupported metric [" + m + "]. " + - "Supported metrics include: " + RollupField.SUPPORTED_METRICS); - } - }); - return new MetricConfig(field, metrics); - } + public static MetricConfig fromXContent(final XContentParser parser) throws IOException { + return PARSER.parse(parser, null); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfig.java index 422ecdd5fd9fb..1abec72ef5360 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfig.java @@ -63,7 +63,7 @@ public class RollupJobConfig implements NamedWriteable, ToXContentObject { static { PARSER.declareString(RollupJobConfig.Builder::setId, RollupField.ID); PARSER.declareObject(RollupJobConfig.Builder::setGroupConfig, (p, c) -> GroupConfig.PARSER.apply(p,c).build(), GROUPS); - PARSER.declareObjectArray(RollupJobConfig.Builder::setMetricsConfig, (p, c) -> MetricConfig.PARSER.apply(p, c).build(), METRICS); + 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); @@ -160,10 +160,8 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par } if (metricsConfig != null) { builder.startArray(METRICS.getPreferredName()); - for (MetricConfig config : metricsConfig) { - builder.startObject(); - config.toXContent(builder, params); - builder.endObject(); + for (MetricConfig metric : metricsConfig) { + metric.toXContent(builder, params); } builder.endArray(); } 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 36b15a1e1f5da..603faafdc369d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java @@ -17,6 +17,7 @@ import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Random; import java.util.stream.Collectors; @@ -38,11 +39,7 @@ public static RollupJobConfig.Builder getRollupJob(String jobId) { builder.setGroupConfig(ConfigTestHelpers.getGroupConfig().build()); builder.setPageSize(ESTestCase.randomIntBetween(1,10)); if (ESTestCase.randomBoolean()) { - List metrics = IntStream.range(1, ESTestCase.randomIntBetween(1,10)) - .mapToObj(n -> ConfigTestHelpers.getMetricConfig().build()) - .collect(Collectors.toList()); - - builder.setMetricsConfig(metrics); + builder.setMetricsConfig(randomMetricsConfigs(ESTestCase.random())); } return builder; } @@ -59,32 +56,6 @@ public static GroupConfig.Builder getGroupConfig() { return groupBuilder; } - public static MetricConfig.Builder getMetricConfig() { - MetricConfig.Builder builder = new MetricConfig.Builder(); - builder.setField(ESTestCase.randomAlphaOfLength(15)); // large names so we don't accidentally collide - List metrics = new ArrayList<>(); - if (ESTestCase.randomBoolean()) { - metrics.add("min"); - } - if (ESTestCase.randomBoolean()) { - metrics.add("max"); - } - if (ESTestCase.randomBoolean()) { - metrics.add("sum"); - } - if (ESTestCase.randomBoolean()) { - metrics.add("avg"); - } - if (ESTestCase.randomBoolean()) { - metrics.add("value_count"); - } - if (metrics.size() == 0) { - metrics.add("min"); - } - builder.setMetrics(metrics); - return builder; - } - 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); @@ -123,6 +94,39 @@ public static HistogramGroupConfig randomHistogramGroupConfig(final Random rando return new HistogramGroupConfig(randomInterval(random), randomFields(random)); } + public static List randomMetricsConfigs(final Random random) { + final int numMetrics = randomIntBetween(random, 1, 10); + final List metrics = new ArrayList<>(numMetrics); + for (int i = 0; i < numMetrics; i++) { + metrics.add(randomMetricConfig(random)); + } + return Collections.unmodifiableList(metrics); + } + + public static MetricConfig randomMetricConfig(final Random random) { + final String field = randomAsciiAlphanumOfLengthBetween(random, 15, 25); // large names so we don't accidentally collide + final List metrics = new ArrayList<>(); + if (random.nextBoolean()) { + metrics.add("min"); + } + if (random.nextBoolean()) { + metrics.add("max"); + } + if (random.nextBoolean()) { + metrics.add("sum"); + } + if (random.nextBoolean()) { + metrics.add("avg"); + } + if (random.nextBoolean()) { + metrics.add("value_count"); + } + if (metrics.size() == 0) { + metrics.add("min"); + } + return new MetricConfig(field, Collections.unmodifiableList(metrics)); + } + public static TermsGroupConfig randomTermsGroupConfig(final Random random) { return new TermsGroupConfig(randomFields(random)); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricsConfigSerializingTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricConfigSerializingTests.java similarity index 69% rename from x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricsConfigSerializingTests.java rename to x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricConfigSerializingTests.java index 9b330e7165093..a5b8d9afead2e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricsConfigSerializingTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricConfigSerializingTests.java @@ -17,14 +17,16 @@ import java.util.HashMap; import java.util.Map; +import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class MetricsConfigSerializingTests extends AbstractSerializingTestCase { +public class MetricConfigSerializingTests extends AbstractSerializingTestCase { + @Override - protected MetricConfig doParseInstance(XContentParser parser) throws IOException { - return MetricConfig.PARSER.apply(parser, null).build(); + protected MetricConfig doParseInstance(final XContentParser parser) throws IOException { + return MetricConfig.fromXContent(parser); } @Override @@ -34,24 +36,20 @@ protected Writeable.Reader instanceReader() { @Override protected MetricConfig createTestInstance() { - return ConfigTestHelpers.getMetricConfig().build(); + return ConfigTestHelpers.randomMetricConfig(random()); } - public void testValidateNoMapping() throws IOException { + public void testValidateNoMapping() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); - MetricConfig config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + MetricConfig config = new MetricConfig("my_field", singletonList("max")); 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.")); } - public void testValidateNomatchingField() throws IOException { - + public void testValidateNomatchingField() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); @@ -59,17 +57,13 @@ public void testValidateNomatchingField() throws IOException { FieldCapabilities fieldCaps = mock(FieldCapabilities.class); responseMap.put("some_other_field", Collections.singletonMap("date", fieldCaps)); - MetricConfig config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + MetricConfig config = new MetricConfig("my_field", singletonList("max")); 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.")); } - public void testValidateFieldWrongType() throws IOException { - + public void testValidateFieldWrongType() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); @@ -77,17 +71,13 @@ public void testValidateFieldWrongType() throws IOException { FieldCapabilities fieldCaps = mock(FieldCapabilities.class); responseMap.put("my_field", Collections.singletonMap("keyword", fieldCaps)); - MetricConfig config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + MetricConfig config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("The field referenced by a metric group must be a [numeric] type, " + "but found [keyword] for field [my_field]")); } - public void testValidateFieldMatchingNotAggregatable() throws IOException { - + public void testValidateFieldMatchingNotAggregatable() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); @@ -96,15 +86,12 @@ public void testValidateFieldMatchingNotAggregatable() throws IOException { when(fieldCaps.isAggregatable()).thenReturn(false); responseMap.put("my_field", Collections.singletonMap("long", fieldCaps)); - MetricConfig config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + MetricConfig config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable across all indices, but is not.")); } - public void testValidateMatchingField() throws IOException { + public void testValidateMatchingField() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); @@ -113,10 +100,7 @@ public void testValidateMatchingField() throws IOException { when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("long", fieldCaps)); - MetricConfig config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + MetricConfig config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); @@ -124,70 +108,49 @@ public void testValidateMatchingField() throws IOException { fieldCaps = mock(FieldCapabilities.class); when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("double", fieldCaps)); - config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); fieldCaps = mock(FieldCapabilities.class); when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("float", fieldCaps)); - config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); fieldCaps = mock(FieldCapabilities.class); when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("short", fieldCaps)); - config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); fieldCaps = mock(FieldCapabilities.class); when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("byte", fieldCaps)); - config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); fieldCaps = mock(FieldCapabilities.class); when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("half_float", fieldCaps)); - config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); fieldCaps = mock(FieldCapabilities.class); when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("scaled_float", fieldCaps)); - config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + config = new MetricConfig("my_field", singletonList("max")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); fieldCaps = mock(FieldCapabilities.class); when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("integer", fieldCaps)); - config = new MetricConfig.Builder() - .setField("my_field") - .setMetrics(Collections.singletonList("max")) - .build(); + config = new MetricConfig("my_field", singletonList("max")); 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 f1ef953c011a1..6574f1e14713e 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 @@ -26,10 +26,10 @@ import org.joda.time.DateTimeZone; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.equalTo; public class RollupJobIdentifierUtilTests extends ESTestCase { @@ -103,10 +103,7 @@ public void testMetricOnlyAgg() { GroupConfig.Builder group = ConfigTestHelpers.getGroupConfig(); group.setDateHisto(new DateHistoGroupConfig.Builder().setField("foo").setInterval(new DateHistogramInterval("1h")).build()); job.setGroupConfig(group.build()); - job.setMetricsConfig(Collections.singletonList(new MetricConfig.Builder() - .setField("bar") - .setMetrics(Collections.singletonList("max")) - .build())); + job.setMetricsConfig(singletonList(new MetricConfig("bar", singletonList("max")))); RollupJobCaps cap = new RollupJobCaps(job.build()); Set caps = singletonSet(cap); @@ -168,10 +165,7 @@ public void testTwoJobsButBothPartialMatches() { GroupConfig.Builder group = ConfigTestHelpers.getGroupConfig(); group.setDateHisto(new DateHistoGroupConfig.Builder().setField("foo").setInterval(new DateHistogramInterval("1h")).build()); job.setGroupConfig(group.build()); - job.setMetricsConfig(Collections.singletonList(new MetricConfig.Builder() - .setField("bar") - .setMetrics(Collections.singletonList("max")) - .build())); + job.setMetricsConfig(singletonList(new MetricConfig("bar", singletonList("max")))); RollupJobCaps cap = new RollupJobCaps(job.build()); Set caps = new HashSet<>(2); caps.add(cap); @@ -180,10 +174,7 @@ public void testTwoJobsButBothPartialMatches() { GroupConfig.Builder group2 = ConfigTestHelpers.getGroupConfig(); group2.setDateHisto(new DateHistoGroupConfig.Builder().setField("foo").setInterval(new DateHistogramInterval("1h")).build()); job2.setGroupConfig(group.build()); - job.setMetricsConfig(Collections.singletonList(new MetricConfig.Builder() - .setField("bar") - .setMetrics(Collections.singletonList("min")) - .build())); + job.setMetricsConfig(singletonList(new MetricConfig("bar", singletonList("min")))); RollupJobCaps cap2 = new RollupJobCaps(job2.build()); caps.add(cap2); @@ -331,12 +322,8 @@ public void testHistoSameNameWrongTypeInCaps() { .build()) .setHisto(new HistogramGroupConfig(1L, "baz")) // <-- NOTE right type but wrong name .build()) - .setMetricsConfig(Arrays.asList(new MetricConfig.Builder() - .setField("max_field") - .setMetrics(Collections.singletonList("max")).build(), - new MetricConfig.Builder() - .setField("avg_field") - .setMetrics(Collections.singletonList("avg")).build())) + .setMetricsConfig( + Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg")))) .build(); Set caps = singletonSet(new RollupJobCaps(job)); @@ -360,12 +347,8 @@ public void testMissingDateHisto() { .setTimeZone(DateTimeZone.UTC) .build()) .build()) - .setMetricsConfig(Arrays.asList(new MetricConfig.Builder() - .setField("max_field") - .setMetrics(Collections.singletonList("max")).build(), - new MetricConfig.Builder() - .setField("avg_field") - .setMetrics(Collections.singletonList("avg")).build())) + .setMetricsConfig( + Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg")))) .build(); Set caps = singletonSet(new RollupJobCaps(job)); @@ -412,12 +395,8 @@ public void testDateHistoMissingFieldInCaps() { .setTimeZone(DateTimeZone.UTC) .build()) .build()) - .setMetricsConfig(Arrays.asList(new MetricConfig.Builder() - .setField("max_field") - .setMetrics(Collections.singletonList("max")).build(), - new MetricConfig.Builder() - .setField("avg_field") - .setMetrics(Collections.singletonList("avg")).build())) + .setMetricsConfig( + Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg")))) .build(); Set caps = singletonSet(new RollupJobCaps(job)); @@ -442,12 +421,8 @@ public void testHistoMissingFieldInCaps() { .build()) .setHisto(new HistogramGroupConfig(1L, "baz")) // <-- NOTE right type but wrong name .build()) - .setMetricsConfig(Arrays.asList(new MetricConfig.Builder() - .setField("max_field") - .setMetrics(Collections.singletonList("max")).build(), - new MetricConfig.Builder() - .setField("avg_field") - .setMetrics(Collections.singletonList("avg")).build())) + .setMetricsConfig( + Arrays.asList(new MetricConfig("max_field", singletonList("max")), new MetricConfig("avg_field", singletonList("avg")))) .build(); Set caps = singletonSet(new RollupJobCaps(job)); @@ -485,9 +460,8 @@ public void testMissingMetric() { int i = ESTestCase.randomIntBetween(0, 3); Set caps = singletonSet(new RollupJobCaps(ConfigTestHelpers - .getRollupJob("foo").setMetricsConfig(Collections.singletonList(new MetricConfig.Builder() - .setField("foo") - .setMetrics(Arrays.asList("avg", "max", "min", "sum")).build())) + .getRollupJob("foo") + .setMetricsConfig(singletonList(new MetricConfig("foo", Arrays.asList("avg", "max", "min", "sum")))) .build())); String aggType; diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java index 9b29c907b3bed..08663eb9bba33 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java @@ -45,6 +45,7 @@ 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,9 +154,8 @@ public void testSimpleMetric() { public void testUnsupportedMetric() { Set caps = singletonSet(new RollupJobCaps(ConfigTestHelpers - .getRollupJob("foo").setMetricsConfig(Collections.singletonList(new MetricConfig.Builder() - .setField("foo") - .setMetrics(Arrays.asList("avg", "max", "min", "sum")).build())) + .getRollupJob("foo") + .setMetricsConfig(singletonList(new MetricConfig("foo", Arrays.asList("avg", "max", "min", "sum")))) .build())); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, @@ -384,7 +384,7 @@ public void testUnsupportedAgg() { assertThat(e.getMessage(), equalTo("Unable to translate aggregation tree into Rollup. Aggregation [test_geo] is of type " + "[GeoDistanceAggregationBuilder] which is currently unsupported.")); } - + private Set singletonSet(RollupJobCaps cap) { Set caps = new HashSet<>(); caps.add(cap); 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 5b2f22dcd2db2..9d8eb4dae09f8 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 @@ -454,7 +454,7 @@ public void testTwoMatchingJobs() { job2.setGroupConfig(group.build()); // so that the jobs aren't exactly equal - job2.setMetricsConfig(Collections.singletonList(ConfigTestHelpers.getMetricConfig().build())); + job2.setMetricsConfig(ConfigTestHelpers.randomMetricsConfigs(random())); RollupJobCaps cap2 = new RollupJobCaps(job2.build()); Set caps = new HashSet<>(2); 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 d5fd300b68747..df389cc753948 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 @@ -16,35 +16,29 @@ import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import org.joda.time.DateTimeZone; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.Map; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.equalTo; - +//TODO split this into dedicated unit test classes (one for each config object) public class ConfigTests extends ESTestCase { public void testEmptyField() { - MetricConfig.Builder config = ConfigTestHelpers.getMetricConfig(); - config.setField(null); - Exception e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [field] must be a non-null, non-empty string.")); + Exception e = expectThrows(IllegalArgumentException.class, () -> new MetricConfig(null, singletonList("max"))); + assertThat(e.getMessage(), equalTo("Field must be a non-null, non-empty string")); - config.setField(""); - e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [field] must be a non-null, non-empty string.")); + e = expectThrows(IllegalArgumentException.class, () -> new MetricConfig("", singletonList("max"))); + assertThat(e.getMessage(), equalTo("Field must be a non-null, non-empty string")); } public void testEmptyMetrics() { - MetricConfig.Builder config = ConfigTestHelpers.getMetricConfig(); - config.setMetrics(null); - Exception e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [metrics] must be a non-null, non-empty array of strings.")); + Exception e = expectThrows(IllegalArgumentException.class, () -> new MetricConfig("foo", emptyList())); + assertThat(e.getMessage(), equalTo("Metrics must be a non-null, non-empty array of strings")); - config.setMetrics(Collections.emptyList()); - e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Parameter [metrics] must be a non-null, non-empty array of strings.")); + e = expectThrows(IllegalArgumentException.class, () -> new MetricConfig("foo", null)); + assertThat(e.getMessage(), equalTo("Metrics must be a non-null, non-empty array of strings")); } public void testEmptyGroup() { @@ -233,11 +227,4 @@ public void testNoHeadersInJSON() { assertFalse(json.contains("authentication")); assertFalse(json.contains("security")); } - - public void testUnsupportedMetric() { - MetricConfig.Builder config = ConfigTestHelpers.getMetricConfig(); - config.setMetrics(Arrays.asList("max","foo")); - Exception e = expectThrows(IllegalArgumentException.class, config::build); - assertThat(e.getMessage(), equalTo("Unsupported metric [foo]. Supported metrics include: [max, min, sum, avg, value_count]")); - } } 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 f36ef3b00c70d..72d37756572f9 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 @@ -54,6 +54,7 @@ import java.util.Map; import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomHistogramGroupConfig; +import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -102,10 +103,7 @@ public void testMissingFields() throws IOException { .build(); CompositeAggregationBuilder compositeBuilder = new CompositeAggregationBuilder(RollupIndexer.AGGREGATION_NAME, dateHistoGroupConfig.toBuilders()); - MetricConfig metricConfig = new MetricConfig.Builder() - .setField("does_not_exist") - .setMetrics(Collections.singletonList("max")) - .build(); + MetricConfig metricConfig = new MetricConfig("does_not_exist", singletonList("max")); metricConfig.toBuilders().forEach(compositeBuilder::subAggregation); Aggregator aggregator = createAggregator(compositeBuilder, indexSearcher, timestampFieldType, valueFieldType); @@ -170,9 +168,9 @@ public void testCorrectFields() throws IOException { .interval(1); CompositeAggregationBuilder compositeBuilder = new CompositeAggregationBuilder(RollupIndexer.AGGREGATION_NAME, - Collections.singletonList(dateHisto)); + singletonList(dateHisto)); - MetricConfig metricConfig = new MetricConfig.Builder().setField(valueField).setMetrics(Collections.singletonList("max")).build(); + MetricConfig metricConfig = new MetricConfig(valueField, singletonList("max")); metricConfig.toBuilders().forEach(compositeBuilder::subAggregation); Aggregator aggregator = createAggregator(compositeBuilder, indexSearcher, timestampFieldType, valueFieldType); @@ -226,9 +224,9 @@ public void testNumericTerms() throws IOException { TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("the_terms." + TermsAggregationBuilder.NAME).field(valueField); CompositeAggregationBuilder compositeBuilder = new CompositeAggregationBuilder(RollupIndexer.AGGREGATION_NAME, - Collections.singletonList(terms)); + singletonList(terms)); - MetricConfig metricConfig = new MetricConfig.Builder().setField(valueField).setMetrics(Collections.singletonList("max")).build(); + MetricConfig metricConfig = new MetricConfig(valueField, singletonList("max")); metricConfig.toBuilders().forEach(compositeBuilder::subAggregation); Aggregator aggregator = createAggregator(compositeBuilder, indexSearcher, valueFieldType); @@ -292,9 +290,9 @@ public void testEmptyCounts() throws IOException { .dateHistogramInterval(new DateHistogramInterval("1d")); CompositeAggregationBuilder compositeBuilder = new CompositeAggregationBuilder(RollupIndexer.AGGREGATION_NAME, - Collections.singletonList(dateHisto)); + singletonList(dateHisto)); - MetricConfig metricConfig = new MetricConfig.Builder().setField("another_field").setMetrics(Arrays.asList("avg", "sum")).build(); + MetricConfig metricConfig = new MetricConfig("another_field", Arrays.asList("avg", "sum")); metricConfig.toBuilders().forEach(compositeBuilder::subAggregation); Aggregator aggregator = createAggregator(compositeBuilder, indexSearcher, timestampFieldType, valueFieldType); @@ -439,7 +437,7 @@ public void testMissingBuckets() throws IOException { CompositeAggregationBuilder compositeBuilder = new CompositeAggregationBuilder(RollupIndexer.AGGREGATION_NAME, termsGroupConfig.toBuilders()).size(numDocs*2); - MetricConfig metricConfig = new MetricConfig.Builder().setField(metricField).setMetrics(Collections.singletonList("max")).build(); + MetricConfig metricConfig = new MetricConfig(metricField, singletonList("max")); metricConfig.toBuilders().forEach(compositeBuilder::subAggregation); Aggregator aggregator = createAggregator(compositeBuilder, indexSearcher, valueFieldType, metricFieldType); diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java index f658fa574eb99..5fb7f654f36bc 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java @@ -47,13 +47,13 @@ import org.elasticsearch.search.aggregations.bucket.composite.CompositeAggregation; import org.elasticsearch.search.aggregations.bucket.composite.CompositeAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; +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.IndexerState; 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.ConfigTestHelpers; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.junit.Before; @@ -143,7 +143,7 @@ public void testDateHistoAndMetrics() throws Exception { DateHistoGroupConfig dateHistoConfig = new DateHistoGroupConfig.Builder() .setField(field) .setInterval(new DateHistogramInterval("1h")).build(); - MetricConfig config = new MetricConfig.Builder().setField("counter").setMetrics(Arrays.asList("avg", "sum", "max", "min")).build(); + MetricConfig config = new MetricConfig("counter", Arrays.asList("avg", "sum", "max", "min")); RollupJobConfig job = createJob(rollupIndex, new GroupConfig.Builder().setDateHisto(dateHistoConfig).build(), Collections.singletonList(config)); final List> dataset = new ArrayList<>(); @@ -417,7 +417,7 @@ public void testRandomizedDateHisto() throws Exception { DateHistoGroupConfig dateHistoConfig = new DateHistoGroupConfig.Builder() .setField(timestampField) .setInterval(new DateHistogramInterval(timeInterval)).build(); - MetricConfig metricConfig = new MetricConfig.Builder().setField(valueField).setMetrics(Collections.singletonList("avg")).build(); + MetricConfig metricConfig = new MetricConfig(valueField, Collections.singletonList("avg")); RollupJobConfig job = createJob(rollupIndex, new GroupConfig.Builder().setDateHisto(dateHistoConfig).build(), Collections.singletonList(metricConfig));