Skip to content

[Rollup] Remove builders from HistoGroupConfig #32533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 2, 2018
Merged

[Rollup] Remove builders from HistoGroupConfig #32533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f79d18a
[Rollup] Remove builders from HistoGroupConfig
tlrx Aug 1, 2018
377e39a
remove randomization
tlrx Aug 1, 2018
3b14d4d
Fix compilation error
tlrx Aug 1, 2018
a8168b3
Apply feedback
tlrx Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/GroupConfig.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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