Skip to content

Commit

Permalink
[Rollup] Remove builders from MetricConfig (#32536)
Browse files Browse the repository at this point in the history 
Related to #29827
  • Loading branch information
@tlrx
tlrx committed Aug 3, 2018
1 parent f809d6f commit 937dcfd
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 253 deletions.
126 changes: 52 additions & 74 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
*
Expand All @@ -48,14 +51,7 @@
* ]
* }
*/
public class MetricConfig implements Writeable, ToXContentFragment {
private static final String NAME = "metric_config";

private String field;
private List<String> 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");
Expand All @@ -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<MetricConfig.Builder, Void> 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<MetricConfig, Void> PARSER;
static {
PARSER.declareString(MetricConfig.Builder::setField, FIELD);
PARSER.declareStringArray(MetricConfig.Builder::setMetrics, METRICS);
PARSER = new ConstructingObjectParser<>(NAME, args -> {
@SuppressWarnings("unchecked") List<String> metrics = (List<String>) 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<String> metrics) {
this.field = name;
private final String field;
private final List<String> metrics;

public MetricConfig(final String field, final List<String> 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<String> getMetrics() {
return metrics;
}
Expand Down Expand Up @@ -159,10 +182,13 @@ public void validateMappings(Map<String, Map<String, FieldCapabilities>> 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
Expand All @@ -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
Expand All @@ -197,52 +220,7 @@ public String toString() {
return Strings.toString(this, true, true);
}


public static class Builder {
private String field;
private List<String> 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<String> getMetrics() {
return metrics;
}

public MetricConfig.Builder setMetrics(List<String> 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);
}
}
8 changes: 3 additions & 5 deletions ...ck/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/RollupJobConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
Expand Down
66 changes: 35 additions & 31 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<MetricConfig> 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;
}
Expand All @@ -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<String> 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);
Expand Down Expand Up @@ -123,6 +94,39 @@ public static HistogramGroupConfig randomHistogramGroupConfig(final Random rando
return new HistogramGroupConfig(randomInterval(random), randomFields(random));
}

public static List<MetricConfig> randomMetricsConfigs(final Random random) {
final int numMetrics = randomIntBetween(random, 1, 10);
final List<MetricConfig> 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<String> 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));
}
Expand Down
Loading

0 comments on commit 937dcfd

Please sign in to comment.