Skip to content

Commit

Permalink
[Rollup] Remove builders from GroupConfig (#32614)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tlrx
tlrx committed Aug 7, 2018
1 parent 7f774ed commit 0061c34
Show file tree
Hide file tree
Showing 16 changed files with 245 additions and 330 deletions.
10 changes: 5 additions & 5 deletions ...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/RollupJobCaps.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,18 @@ public RollupJobCaps(RollupJobConfig job) {
jobID = job.getId();
rollupIndex = job.getRollupIndex();
indexPattern = job.getIndexPattern();
Map<String, Object> dateHistoAggCap = job.getGroupConfig().getDateHisto().toAggCap();
String dateField = job.getGroupConfig().getDateHisto().getField();
Map<String, Object> dateHistoAggCap = job.getGroupConfig().getDateHistogram().toAggCap();
String dateField = job.getGroupConfig().getDateHistogram().getField();
RollupFieldCaps fieldCaps = fieldCapLookup.get(dateField);
if (fieldCaps == null) {
fieldCaps = new RollupFieldCaps();
}
fieldCaps.addAgg(dateHistoAggCap);
fieldCapLookup.put(dateField, fieldCaps);

if (job.getGroupConfig().getHisto() != null) {
Map<String, Object> histoAggCap = job.getGroupConfig().getHisto().toAggCap();
Arrays.stream(job.getGroupConfig().getHisto().getFields()).forEach(field -> {
if (job.getGroupConfig().getHistogram() != null) {
Map<String, Object> histoAggCap = job.getGroupConfig().getHistogram().toAggCap();
Arrays.stream(job.getGroupConfig().getHistogram().getFields()).forEach(field -> {
RollupFieldCaps caps = fieldCapLookup.get(field);
if (caps == null) {
caps = new RollupFieldCaps();
Expand Down
2 changes: 1 addition & 1 deletion .../core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
*/
public class DateHistogramGroupConfig implements Writeable, ToXContentObject {

private static final String NAME = "date_histogram";
static final String NAME = "date_histogram";
private static final String INTERVAL = "interval";
private static final String FIELD = "field";
public static final String TIME_ZONE = "time_zone";
Expand Down
166 changes: 77 additions & 89 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 @@ -13,17 +13,21 @@
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.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;

import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

import static java.util.Arrays.asList;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;

/**
* The configuration object for the groups section in the rollup config.
Expand All @@ -38,64 +42,85 @@
* }
*/
public class GroupConfig implements Writeable, ToXContentObject {
private static final String NAME = "grouping_config";
private static final ParseField DATE_HISTO = new ParseField("date_histogram");
private static final ParseField HISTO = new ParseField("histogram");
private static final ParseField TERMS = new ParseField("terms");

private final DateHistogramGroupConfig dateHisto;
private final HistogramGroupConfig histo;
private final TermsGroupConfig terms;
public static final String NAME = "groups";
private static final ConstructingObjectParser<GroupConfig, Void> PARSER;
static {
PARSER = new ConstructingObjectParser<>(NAME, args ->
new GroupConfig((DateHistogramGroupConfig) args[0], (HistogramGroupConfig) args[1], (TermsGroupConfig) args[2]));
PARSER.declareObject(constructorArg(),
(p, c) -> DateHistogramGroupConfig.fromXContent(p), new ParseField(DateHistogramGroupConfig.NAME));
PARSER.declareObject(optionalConstructorArg(),
(p, c) -> HistogramGroupConfig.fromXContent(p), new ParseField(HistogramGroupConfig.NAME));
PARSER.declareObject(optionalConstructorArg(),
(p, c) -> TermsGroupConfig.fromXContent(p), new ParseField(TermsGroupConfig.NAME));
}

public static final ObjectParser<GroupConfig.Builder, Void> PARSER = new ObjectParser<>(NAME, GroupConfig.Builder::new);
private final DateHistogramGroupConfig dateHistogram;
private final @Nullable HistogramGroupConfig histogram;
private final @Nullable TermsGroupConfig terms;

static {
PARSER.declareObject(GroupConfig.Builder::setDateHisto, (p,c) -> DateHistogramGroupConfig.fromXContent(p), DATE_HISTO);
PARSER.declareObject(GroupConfig.Builder::setHisto, (p,c) -> HistogramGroupConfig.fromXContent(p), HISTO);
PARSER.declareObject(GroupConfig.Builder::setTerms, (p,c) -> TermsGroupConfig.fromXContent(p), TERMS);
public GroupConfig(final DateHistogramGroupConfig dateHistogram) {
this(dateHistogram, null, null);
}

private GroupConfig(DateHistogramGroupConfig dateHisto, @Nullable HistogramGroupConfig histo, @Nullable TermsGroupConfig terms) {
this.dateHisto = Objects.requireNonNull(dateHisto, "A date_histogram group is mandatory");
this.histo = histo;
public GroupConfig(final DateHistogramGroupConfig dateHistogram,
final @Nullable HistogramGroupConfig histogram,
final @Nullable TermsGroupConfig terms) {
if (dateHistogram == null) {
throw new IllegalArgumentException("Date histogram must not be null");
}
this.dateHistogram = dateHistogram;
this.histogram = histogram;
this.terms = terms;
}

GroupConfig(StreamInput in) throws IOException {
dateHisto = new DateHistogramGroupConfig(in);
histo = in.readOptionalWriteable(HistogramGroupConfig::new);
GroupConfig(final StreamInput in) throws IOException {
dateHistogram = new DateHistogramGroupConfig(in);
histogram = in.readOptionalWriteable(HistogramGroupConfig::new);
terms = in.readOptionalWriteable(TermsGroupConfig::new);
}

public DateHistogramGroupConfig getDateHisto() {
return dateHisto;
/**
* @return the configuration of the date histogram
*/
public DateHistogramGroupConfig getDateHistogram() {
return dateHistogram;
}

public HistogramGroupConfig getHisto() {
return histo;
/**
* @return the configuration of the histogram
*/
@Nullable
public HistogramGroupConfig getHistogram() {
return histogram;
}

/**
* @return the configuration of the terms
*/
@Nullable
public TermsGroupConfig getTerms() {
return terms;
}

public Set<String> getAllFields() {
Set<String> fields = new HashSet<>();
fields.add(dateHisto.getField());
if (histo != null) {
fields.addAll(asList(histo.getFields()));
fields.add(dateHistogram.getField());
if (histogram != null) {
fields.addAll(asList(histogram.getFields()));
}
if (terms != null) {
fields.addAll(asList(terms.getFields()));
}
return fields;
return Collections.unmodifiableSet(fields);
}

public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCapsResponse,
ActionRequestValidationException validationException) {
dateHisto.validateMappings(fieldCapsResponse, validationException);
if (histo != null) {
histo.validateMappings(fieldCapsResponse, validationException);
public void validateMappings(final Map<String, Map<String, FieldCapabilities>> fieldCapsResponse,
final ActionRequestValidationException validationException) {
dateHistogram.validateMappings(fieldCapsResponse, validationException);
if (histogram != null) {
histogram.validateMappings(fieldCapsResponse, validationException);
}
if (terms != null) {
terms.validateMappings(fieldCapsResponse, validationException);
Expand All @@ -105,88 +130,51 @@ public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCa
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(DATE_HISTO.getPreferredName(), dateHisto);
if (histo != null) {
builder.field(HISTO.getPreferredName(), histo);
}
if (terms != null) {
builder.field(TERMS.getPreferredName(), terms);
{
builder.field(DateHistogramGroupConfig.NAME, dateHistogram);
if (histogram != null) {
builder.field(HistogramGroupConfig.NAME, histogram);
}
if (terms != null) {
builder.field(TermsGroupConfig.NAME, terms);
}
}
builder.endObject();
return builder;
return builder.endObject();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
dateHisto.writeTo(out);
out.writeOptionalWriteable(histo);
public void writeTo(final StreamOutput out) throws IOException {
dateHistogram.writeTo(out);
out.writeOptionalWriteable(histogram);
out.writeOptionalWriteable(terms);
}

@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;
}

GroupConfig that = (GroupConfig) other;

return Objects.equals(this.dateHisto, that.dateHisto)
&& Objects.equals(this.histo, that.histo)
&& Objects.equals(this.terms, that.terms);
final GroupConfig that = (GroupConfig) other;
return Objects.equals(dateHistogram, that.dateHistogram)
&& Objects.equals(histogram, that.histogram)
&& Objects.equals(terms, that.terms);
}

@Override
public int hashCode() {
return Objects.hash(dateHisto, histo, terms);
return Objects.hash(dateHistogram, histogram, terms);
}

@Override
public String toString() {
return Strings.toString(this, true, true);
}

public static class Builder {
private DateHistogramGroupConfig dateHisto;
private HistogramGroupConfig histo;
private TermsGroupConfig terms;

public DateHistogramGroupConfig getDateHisto() {
return dateHisto;
}

public GroupConfig.Builder setDateHisto(DateHistogramGroupConfig dateHisto) {
this.dateHisto = dateHisto;
return this;
}

public HistogramGroupConfig getHisto() {
return histo;
}

public GroupConfig.Builder setHisto(HistogramGroupConfig histo) {
this.histo = histo;
return this;
}

public TermsGroupConfig getTerms() {
return terms;
}

public GroupConfig.Builder setTerms(TermsGroupConfig terms) {
this.terms = terms;
return this;
}

public GroupConfig build() {
if (dateHisto == null) {
throw new IllegalArgumentException("A date_histogram group is mandatory");
}
return new GroupConfig(dateHisto, histo, terms);
}
public static GroupConfig fromXContent(final XContentParser parser) throws IOException {
return PARSER.parse(parser, null);
}
}
2 changes: 1 addition & 1 deletion ...ugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
*/
public class HistogramGroupConfig implements Writeable, ToXContentObject {

public static final String NAME = "histogram";
static final String NAME = "histogram";
private static final String INTERVAL = "interval";
private static final String FIELDS = "fields";
private static final ConstructingObjectParser<HistogramGroupConfig, Void> PARSER;
Expand Down
2 changes: 1 addition & 1 deletion ...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 @@ -62,7 +62,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.declareObject(RollupJobConfig.Builder::setGroupConfig, (p, c) -> GroupConfig.fromXContent(p), GROUPS);
PARSER.declareObjectArray(RollupJobConfig.Builder::setMetricsConfig, (p, c) -> MetricConfig.fromXContent(p), METRICS);
PARSER.declareString((params, val) ->
params.setTimeout(TimeValue.parseTimeValue(val, TIMEOUT.getPreferredName())), TIMEOUT);
Expand Down
2 changes: 1 addition & 1 deletion ...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/TermsGroupConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
*/
public class TermsGroupConfig implements Writeable, ToXContentObject {

private static final String NAME = "terms";
static final String NAME = "terms";
private static final String FIELDS = "fields";

private static final List<String> FLOAT_TYPES = Arrays.asList("half_float", "float", "double", "scaled_float");
Expand Down
17 changes: 6 additions & 11 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 @@ -37,24 +37,19 @@ public static RollupJobConfig.Builder getRollupJob(String jobId) {
String indexPattern = ESTestCase.randomAlphaOfLengthBetween(1,10);
builder.setIndexPattern(indexPattern);
builder.setRollupIndex("rollup_" + indexPattern); // to ensure the index pattern != rollup index
builder.setGroupConfig(ConfigTestHelpers.getGroupConfig().build());
builder.setGroupConfig(ConfigTestHelpers.randomGroupConfig(ESTestCase.random()));
builder.setPageSize(ESTestCase.randomIntBetween(1,10));
if (ESTestCase.randomBoolean()) {
builder.setMetricsConfig(randomMetricsConfigs(ESTestCase.random()));
}
return builder;
}

public static GroupConfig.Builder getGroupConfig() {
GroupConfig.Builder groupBuilder = new GroupConfig.Builder();
groupBuilder.setDateHisto(randomDateHistogramGroupConfig(ESTestCase.random()));
if (ESTestCase.randomBoolean()) {
groupBuilder.setHisto(randomHistogramGroupConfig(ESTestCase.random()));
}
if (ESTestCase.randomBoolean()) {
groupBuilder.setTerms(randomTermsGroupConfig(ESTestCase.random()));
}
return groupBuilder;
public static GroupConfig randomGroupConfig(final Random random) {
DateHistogramGroupConfig dateHistogram = randomDateHistogramGroupConfig(random);
HistogramGroupConfig histogram = random.nextBoolean() ? randomHistogramGroupConfig(random) : null;
TermsGroupConfig terms = random.nextBoolean() ? randomTermsGroupConfig(random) : null;
return new GroupConfig(dateHistogram, histogram, terms);
}

private static final String[] TIME_SUFFIXES = new String[]{"d", "h", "ms", "s", "m"};
Expand Down
10 changes: 6 additions & 4 deletions ...re/src/test/java/org/elasticsearch/xpack/core/rollup/job/GroupConfigSerializingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@
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 java.io.IOException;

import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomGroupConfig;

public class GroupConfigSerializingTests extends AbstractSerializingTestCase<GroupConfig> {

@Override
protected GroupConfig doParseInstance(XContentParser parser) throws IOException {
return GroupConfig.PARSER.apply(parser, null).build();
protected GroupConfig doParseInstance(final XContentParser parser) throws IOException {
return GroupConfig.fromXContent(parser);
}

@Override
Expand All @@ -25,6 +27,6 @@ protected Writeable.Reader<GroupConfig> instanceReader() {

@Override
protected GroupConfig createTestInstance() {
return ConfigTestHelpers.getGroupConfig().build();
return randomGroupConfig(random());
}
}
6 changes: 3 additions & 3 deletions x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/IndexerUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ private static void processKeys(Map<String, Object> keys, Map<String, Object> do
if (k.endsWith("." + DateHistogramAggregationBuilder.NAME)) {
assert v != null;
doc.put(k + "." + RollupField.TIMESTAMP, v);
doc.put(k + "." + RollupField.INTERVAL, groupConfig.getDateHisto().getInterval());
doc.put(k + "." + DateHistogramGroupConfig.TIME_ZONE, groupConfig.getDateHisto().getTimeZone().toString());
doc.put(k + "." + RollupField.INTERVAL, groupConfig.getDateHistogram().getInterval());
doc.put(k + "." + DateHistogramGroupConfig.TIME_ZONE, groupConfig.getDateHistogram().getTimeZone());
idGenerator.add((Long)v);
} else if (k.endsWith("." + HistogramAggregationBuilder.NAME)) {
doc.put(k + "." + RollupField.VALUE, v);
doc.put(k + "." + RollupField.INTERVAL, groupConfig.getHisto().getInterval());
doc.put(k + "." + RollupField.INTERVAL, groupConfig.getHistogram().getInterval());
if (v == null) {
idGenerator.addNull();
} else {
Expand Down
Loading

0 comments on commit 0061c34

Please sign in to comment.