Skip to content

Commit

Permalink
[Rollup] Remove builders from HistoGroupConfig (#32533)
Browse files Browse the repository at this point in the history 
Related to #29827
  • Loading branch information
@tlrx
tlrx authored Aug 2, 2018
1 parent 4cdbb42 commit 08e4f4b
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 143 deletions.
22 changes: 10 additions & 12 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,34 +44,34 @@ 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<GroupConfig.Builder, Void> 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;
}

GroupConfig(StreamInput in) throws IOException {
dateHisto = new DateHistoGroupConfig(in);
histo = in.readOptionalWriteable(HistoGroupConfig::new);
histo = in.readOptionalWriteable(HistogramGroupConfig::new);
terms = in.readOptionalWriteable(TermsGroupConfig::new);
}

public DateHistoGroupConfig getDateHisto() {
return dateHisto;
}

public HistoGroupConfig getHisto() {
public HistogramGroupConfig getHisto() {
return histo;
}

Expand All @@ -83,7 +83,7 @@ public Set<String> getAllFields() {
Set<String> 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()));
Expand All @@ -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);
Expand Down Expand Up @@ -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() {
Expand All @@ -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;
}
Expand Down
101 changes: 39 additions & 62 deletions ...ack/core/rollup/job/HistoGroupConfig.java → ...core/rollup/job/HistogramGroupConfig.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.bucket.composite.CompositeValuesSourceBuilder;
import org.elasticsearch.search.aggregations.bucket.composite.HistogramValuesSourceBuilder;
import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder;
Expand All @@ -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
*
Expand All @@ -42,28 +44,36 @@
* ]
* }
*/
public class HistoGroupConfig implements Writeable, ToXContentFragment {
private static final String NAME = "histo_group_config";
public static final ObjectParser<HistoGroupConfig.Builder, Void> 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<HistogramGroupConfig, Void> PARSER;
static {
PARSER = new ConstructingObjectParser<>(NAME, args -> {
@SuppressWarnings("unchecked") List<String> fields = (List<String>) 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();
}
Expand Down Expand Up @@ -101,18 +111,14 @@ public List<CompositeValuesSourceBuilder<?>> toBuilders() {
public Map<String, Object> toAggCap() {
Map<String, Object> map = new HashMap<>(2);
map.put("agg", HistogramAggregationBuilder.NAME);
map.put(INTERVAL.getPreferredName(), interval);
map.put(INTERVAL, interval);
return map;
}

public Map<String, Object> getMetadata() {
return Collections.singletonMap(RollupField.formatMetaField(RollupField.INTERVAL), interval);
}

public Set<String> getAllFields() {
return Arrays.stream(fields).collect(Collectors.toSet());
}

public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCapsResponse,
ActionRequestValidationException validationException) {

Expand All @@ -138,9 +144,13 @@ public void validateMappings(Map<String, Map<String, FieldCapabilities>> 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;
}

Expand All @@ -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
Expand All @@ -176,36 +182,7 @@ public String toString() {
return Strings.toString(this, true, true);
}

public static class Builder {
private long interval = 0;
private List<String> fields;

public long getInterval() {
return interval;
}

public HistoGroupConfig.Builder setInterval(long interval) {
this.interval = interval;
return this;
}

public List<String> getFields() {
return fields;
}

public HistoGroupConfig.Builder setFields(List<String> 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);
}
}
20 changes: 11 additions & 9 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 @@ -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;
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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<String> getFields() {
return IntStream.range(0, ESTestCase.randomIntBetween(1, 10))
.mapToObj(n -> ESTestCase.randomAlphaOfLengthBetween(5, 10))
Expand All @@ -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));
}
Expand All @@ -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);
}
}
Loading

0 comments on commit 08e4f4b

Please sign in to comment.