Skip to content

Commit 6f0e3fb

Browse files
talevytalevy
committed
refactor sourceIndex out of RollupV2Config (#65478)
this commit extracts the sourceIndex from the RollupV2Config. It was not a natural fit. The sourceIndex will now be a part of the RollupAction.Request. renamed RollupV2Config to RollupActionConfig relates #42720.
1 parent 0085361 commit 6f0e3fb

File tree

7 files changed

+71
-91
lines changed

7 files changed

+71
-91
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/v2/RollupAction.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,20 @@ private RollupAction() {
3434
}
3535

3636
public static class Request extends ActionRequest implements ToXContentObject {
37-
private RollupV2Config rollupConfig;
37+
private String sourceIndex;
38+
private RollupActionConfig rollupConfig;
3839

39-
public Request(RollupV2Config rollupConfig) {
40+
public Request(String sourceIndex, RollupActionConfig rollupConfig) {
41+
this.sourceIndex = sourceIndex;
4042
this.rollupConfig = rollupConfig;
4143
}
4244

4345
public Request() {}
4446

4547
public Request(StreamInput in) throws IOException {
4648
super(in);
47-
rollupConfig = new RollupV2Config(in);
49+
sourceIndex = in.readString();
50+
rollupConfig = new RollupActionConfig(in);
4851
}
4952

5053
@Override
@@ -55,10 +58,15 @@ public Task createTask(long id, String type, String action, TaskId parentTaskId,
5558
@Override
5659
public void writeTo(StreamOutput out) throws IOException {
5760
super.writeTo(out);
61+
out.writeString(sourceIndex);
5862
rollupConfig.writeTo(out);
5963
}
6064

61-
public RollupV2Config getRollupConfig() {
65+
public String getSourceIndex() {
66+
return sourceIndex;
67+
}
68+
69+
public RollupActionConfig getRollupConfig() {
6270
return rollupConfig;
6371
}
6472

@@ -70,14 +78,15 @@ public ActionRequestValidationException validate() {
7078
@Override
7179
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
7280
builder.startObject();
81+
builder.field("source_index", sourceIndex);
7382
rollupConfig.toXContent(builder, params);
7483
builder.endObject();
7584
return builder;
7685
}
7786

7887
@Override
7988
public int hashCode() {
80-
return Objects.hash(rollupConfig);
89+
return Objects.hash(sourceIndex, rollupConfig);
8190
}
8291

8392
@Override
@@ -89,7 +98,8 @@ public boolean equals(Object obj) {
8998
return false;
9099
}
91100
Request other = (Request) obj;
92-
return Objects.equals(rollupConfig, other.rollupConfig);
101+
return Objects.equals(sourceIndex, other.sourceIndex)
102+
&& Objects.equals(rollupConfig, other.rollupConfig);
93103
}
94104
}
95105

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/v2/RollupV2Config.java‎ renamed to ‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/v2/RollupActionConfig.java‎

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@
3535
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
3636

3737
/**
38-
* This class holds the configuration details of a rollup V2 job, such as the groupings, metrics, what
38+
* This class holds the configuration details of a {@link RollupAction} job, such as the groupings, metrics, what
3939
* index to rollup and where to roll them to.
4040
*/
41-
public class RollupV2Config implements NamedWriteable, ToXContentObject {
41+
public class RollupActionConfig implements NamedWriteable, ToXContentObject {
4242

43-
private static final String NAME = "xpack/rollupv2/config";
43+
private static final String NAME = "xpack/rollup/action/config";
4444
private static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(20);
4545
private static final String TIMEOUT = "timeout";
4646
private static final String ROLLUP_INDEX = "rollup_index";
@@ -49,17 +49,16 @@ public class RollupV2Config implements NamedWriteable, ToXContentObject {
4949
private final List<MetricConfig> metricsConfig;
5050
private final TimeValue timeout;
5151
private String rollupIndex;
52-
private String sourceIndex;
5352

54-
private static final ConstructingObjectParser<RollupV2Config, String> PARSER;
53+
private static final ConstructingObjectParser<RollupActionConfig, Void> PARSER;
5554
static {
56-
PARSER = new ConstructingObjectParser<>(NAME, false, (args, sourceIndex) -> {
55+
PARSER = new ConstructingObjectParser<>(NAME, false, (args) -> {
5756
String rollupIndex = (String) args[0];
5857
GroupConfig groupConfig = (GroupConfig) args[1];
5958
@SuppressWarnings("unchecked")
6059
List<MetricConfig> metricsConfig = (List<MetricConfig>) args[2];
6160
TimeValue timeout = (TimeValue) args[3];
62-
return new RollupV2Config(sourceIndex, groupConfig, metricsConfig, timeout, rollupIndex);
61+
return new RollupActionConfig(groupConfig, metricsConfig, timeout, rollupIndex);
6362
});
6463
PARSER.declareString(constructorArg(), new ParseField(ROLLUP_INDEX));
6564
PARSER.declareObject(optionalConstructorArg(), (p, c) -> GroupConfig.fromXContent(p), new ParseField(GroupConfig.NAME));
@@ -68,42 +67,29 @@ public class RollupV2Config implements NamedWriteable, ToXContentObject {
6867
new ParseField(TIMEOUT), ObjectParser.ValueType.STRING_OR_NULL);
6968
}
7069

71-
public RollupV2Config(String sourceIndex, final GroupConfig groupConfig, final List<MetricConfig> metricsConfig,
72-
final @Nullable TimeValue timeout, final String rollupIndex) {
73-
if (sourceIndex == null || sourceIndex.isEmpty()) {
74-
throw new IllegalArgumentException("The source index must be a non-null, non-empty string");
75-
}
70+
public RollupActionConfig(final GroupConfig groupConfig, final List<MetricConfig> metricsConfig,
71+
final @Nullable TimeValue timeout, final String rollupIndex) {
7672
if (rollupIndex == null || rollupIndex.isEmpty()) {
7773
throw new IllegalArgumentException("Rollup index must be a non-null, non-empty string");
7874
}
7975
if (groupConfig == null && (metricsConfig == null || metricsConfig.isEmpty())) {
8076
throw new IllegalArgumentException("At least one grouping or metric must be configured");
8177
}
82-
this.sourceIndex = sourceIndex;
8378
this.rollupIndex = rollupIndex;
8479
this.groupConfig = groupConfig;
8580
this.metricsConfig = metricsConfig != null ? metricsConfig : Collections.emptyList();
8681
this.timeout = timeout != null ? timeout : DEFAULT_TIMEOUT;
8782
}
8883

89-
public RollupV2Config(final StreamInput in) throws IOException {
90-
this.sourceIndex = in.readString();
84+
public RollupActionConfig(final StreamInput in) throws IOException {
9185
rollupIndex = in.readString();
9286
groupConfig = in.readOptionalWriteable(GroupConfig::new);
9387
metricsConfig = in.readList(MetricConfig::new);
9488
timeout = in.readTimeValue();
9589
}
9690

9791
public String getId() {
98-
return RollupField.NAME + "_" + sourceIndex + "_" + rollupIndex;
99-
}
100-
101-
public String getSourceIndex() {
102-
return sourceIndex;
103-
}
104-
105-
public void setSourceIndex(String sourceIndex) {
106-
this.sourceIndex = sourceIndex;
92+
return RollupField.NAME + "_" + rollupIndex;
10793
}
10894

10995
public void setRollupIndex(String rollupIndex) {
@@ -177,7 +163,6 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa
177163

178164
@Override
179165
public void writeTo(final StreamOutput out) throws IOException {
180-
out.writeString(sourceIndex);
181166
out.writeString(rollupIndex);
182167
out.writeOptionalWriteable(groupConfig);
183168
out.writeList(metricsConfig);
@@ -193,17 +178,16 @@ public boolean equals(Object other) {
193178
return false;
194179
}
195180

196-
final RollupV2Config that = (RollupV2Config) other;
197-
return Objects.equals(this.sourceIndex, that.sourceIndex)
198-
&& Objects.equals(this.rollupIndex, that.rollupIndex)
181+
final RollupActionConfig that = (RollupActionConfig) other;
182+
return Objects.equals(this.rollupIndex, that.rollupIndex)
199183
&& Objects.equals(this.groupConfig, that.groupConfig)
200184
&& Objects.equals(this.metricsConfig, that.metricsConfig)
201185
&& Objects.equals(this.timeout, that.timeout);
202186
}
203187

204188
@Override
205189
public int hashCode() {
206-
return Objects.hash(sourceIndex, rollupIndex, groupConfig, metricsConfig, timeout);
190+
return Objects.hash(rollupIndex, groupConfig, metricsConfig, timeout);
207191
}
208192

209193
@Override
@@ -218,7 +202,7 @@ public String toJSONString() {
218202
return toString();
219203
}
220204

221-
public static RollupV2Config fromXContent(final XContentParser parser, final String sourceIndex) throws IOException {
222-
return PARSER.parse(parser, sourceIndex);
205+
public static RollupActionConfig fromXContent(final XContentParser parser) throws IOException {
206+
return PARSER.parse(parser, null);
223207
}
224208
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/v2/RollupTask.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@
2121
public class RollupTask extends CancellableTask {
2222
private static final Logger logger = LogManager.getLogger(RollupTask.class.getName());
2323

24-
private RollupV2Config config;
24+
private RollupActionConfig config;
2525
private RollupJobStatus status;
2626

27-
RollupTask(long id, String type, String action, TaskId parentTask, RollupV2Config config, Map<String, String> headers) {
27+
RollupTask(long id, String type, String action, TaskId parentTask, RollupActionConfig config, Map<String, String> headers) {
2828
super(id, type, action, RollupField.NAME + "_" + config.getRollupIndex(), parentTask, headers);
2929
this.config = config;
3030
}
3131

32-
public RollupV2Config config() {
32+
public RollupActionConfig config() {
3333
return config;
3434
}
3535

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/v2/RollupV2ConfigTests.java‎ renamed to ‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/v2/RollupActionConfigTests.java‎

Lines changed: 14 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
1313
import org.elasticsearch.xpack.core.rollup.job.GroupConfig;
1414
import org.elasticsearch.xpack.core.rollup.job.MetricConfig;
15-
import org.junit.Before;
1615

1716
import java.io.IOException;
1817
import java.util.List;
@@ -22,58 +21,43 @@
2221
import static org.hamcrest.Matchers.equalTo;
2322

2423

25-
public class RollupV2ConfigTests extends AbstractSerializingTestCase<RollupV2Config> {
26-
27-
String sourceIndex;
28-
29-
@Before
30-
void setUpSourceIndex() {
31-
sourceIndex = randomAlphaOfLength(10);
32-
}
24+
public class RollupActionConfigTests extends AbstractSerializingTestCase<RollupActionConfig> {
3325

3426
@Override
35-
protected RollupV2Config createTestInstance() {
36-
return randomConfig(random(), sourceIndex);
27+
protected RollupActionConfig createTestInstance() {
28+
return randomConfig(random());
3729
}
3830

39-
public static RollupV2Config randomConfig(Random random, String sourceIndex) {
40-
final String rollupIndex = "rollup-" + sourceIndex;
31+
public static RollupActionConfig randomConfig(Random random) {
32+
final String rollupIndex = randomAlphaOfLength(5);
4133
final TimeValue timeout = random.nextBoolean() ? null : ConfigTestHelpers.randomTimeout(random);
4234
final GroupConfig groupConfig = ConfigTestHelpers.randomGroupConfig(random);
4335
final List<MetricConfig> metricConfigs = ConfigTestHelpers.randomMetricsConfigs(random);
44-
return new RollupV2Config(sourceIndex, groupConfig, metricConfigs, timeout, rollupIndex);
36+
return new RollupActionConfig(groupConfig, metricConfigs, timeout, rollupIndex);
4537
}
4638

4739
@Override
48-
protected Writeable.Reader<RollupV2Config> instanceReader() {
49-
return RollupV2Config::new;
40+
protected Writeable.Reader<RollupActionConfig> instanceReader() {
41+
return RollupActionConfig::new;
5042
}
5143

5244
@Override
53-
protected RollupV2Config doParseInstance(final XContentParser parser) throws IOException {
54-
return RollupV2Config.fromXContent(parser, sourceIndex);
55-
}
56-
57-
public void testEmptySourceIndex() {
58-
final RollupV2Config sample = createTestInstance();
59-
Exception e = expectThrows(IllegalArgumentException.class, () ->
60-
new RollupV2Config(randomBoolean() ? null : "", sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout(),
61-
sample.getRollupIndex()));
62-
assertThat(e.getMessage(), equalTo("The source index must be a non-null, non-empty string"));
45+
protected RollupActionConfig doParseInstance(final XContentParser parser) throws IOException {
46+
return RollupActionConfig.fromXContent(parser);
6347
}
6448

6549
public void testEmptyRollupIndex() {
66-
final RollupV2Config sample = createTestInstance();
50+
final RollupActionConfig sample = createTestInstance();
6751
Exception e = expectThrows(IllegalArgumentException.class, () ->
68-
new RollupV2Config(sample.getSourceIndex(), sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout(),
52+
new RollupActionConfig(sample.getGroupConfig(), sample.getMetricsConfig(), sample.getTimeout(),
6953
randomBoolean() ? null : ""));
7054
assertThat(e.getMessage(), equalTo("Rollup index must be a non-null, non-empty string"));
7155
}
7256

7357
public void testEmptyGroupAndMetrics() {
74-
final RollupV2Config sample = createTestInstance();
58+
final RollupActionConfig sample = createTestInstance();
7559
Exception e = expectThrows(IllegalArgumentException.class, () ->
76-
new RollupV2Config(sample.getSourceIndex(), null, randomBoolean() ? null : emptyList(), sample.getTimeout(),
60+
new RollupActionConfig(null, randomBoolean() ? null : emptyList(), sample.getTimeout(),
7761
sample.getRollupIndex()));
7862
assertThat(e.getMessage(), equalTo("At least one grouping or metric must be configured"));
7963
}

‎x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/RestRollupAction.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import org.elasticsearch.rest.RestRequest;
1212
import org.elasticsearch.rest.action.RestToXContentListener;
1313
import org.elasticsearch.xpack.core.rollup.v2.RollupAction;
14-
import org.elasticsearch.xpack.core.rollup.v2.RollupV2Config;
14+
import org.elasticsearch.xpack.core.rollup.v2.RollupActionConfig;
1515

1616
import java.io.IOException;
1717
import java.util.Collections;
@@ -29,8 +29,8 @@ public List<Route> routes() {
2929
@Override
3030
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
3131
String index = restRequest.param("index");
32-
RollupV2Config job = RollupV2Config.fromXContent(restRequest.contentParser(), index);
33-
RollupAction.Request request = new RollupAction.Request(job);
32+
RollupActionConfig config = RollupActionConfig.fromXContent(restRequest.contentParser());
33+
RollupAction.Request request = new RollupAction.Request(index, config);
3434
return channel -> client.execute(RollupAction.INSTANCE, request, new RestToXContentListener<>(channel));
3535
}
3636

0 commit comments

Comments
 (0)