From 0de61b74ab0896aeab3ab08110d0d6808fa24134 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 16 Jul 2019 15:11:28 -0700 Subject: [PATCH 1/5] migrate more ML actions off of using Request suppliers many classes still use the Streamable constructors of HandledTransportAction, this commit moves more of those classes to the new Writeable constructors. relates #34389. --- .../action/AbstractTransportGetResourcesAction.java | 5 ++--- .../xpack/core/ml/action/DeleteExpiredDataAction.java | 4 ++++ .../core/ml/action/DeleteModelSnapshotAction.java | 10 +++++++--- .../xpack/core/ml/action/ValidateDetectorAction.java | 8 ++++++-- .../xpack/core/ml/action/ValidateJobConfigAction.java | 8 ++++++-- .../xpack/ml/action/TransportDeleteCalendarAction.java | 2 +- .../ml/action/TransportDeleteCalendarEventAction.java | 2 +- .../ml/action/TransportDeleteExpiredDataAction.java | 2 +- .../xpack/ml/action/TransportDeleteFilterAction.java | 2 +- .../xpack/ml/action/TransportDeleteForecastAction.java | 2 +- .../ml/action/TransportDeleteModelSnapshotAction.java | 2 +- .../ml/action/TransportGetCalendarEventsAction.java | 3 +-- .../ml/action/TransportValidateDetectorAction.java | 2 +- .../ml/action/TransportValidateJobConfigAction.java | 3 +-- 14 files changed, 34 insertions(+), 21 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/AbstractTransportGetResourcesAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/AbstractTransportGetResourcesAction.java index 0a0567923836c..003b702020721 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/AbstractTransportGetResourcesAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/AbstractTransportGetResourcesAction.java @@ -42,7 +42,6 @@ import java.util.List; import java.util.Objects; import java.util.Set; -import java.util.function.Supplier; import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin; @@ -62,8 +61,8 @@ public abstract class AbstractTransportGetResourcesAction request, Client client, NamedXContentRegistry xContentRegistry) { - super(actionName, transportService, request, actionFilters); + Writeable.Reader request, Client client, NamedXContentRegistry xContentRegistry) { + super(actionName, transportService, actionFilters, request); this.client = Objects.requireNonNull(client); this.xContentRegistry = Objects.requireNonNull(xContentRegistry); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteExpiredDataAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteExpiredDataAction.java index 7a294860179b1..cc290bf987e0c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteExpiredDataAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteExpiredDataAction.java @@ -33,6 +33,10 @@ public static class Request extends ActionRequest { public Request() {} + public Request(StreamInput in) throws IOException { + super(in); + } + @Override public ActionRequestValidationException validate() { return null; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteModelSnapshotAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteModelSnapshotAction.java index c97739ac77ee8..8f8b2af9700e0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteModelSnapshotAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteModelSnapshotAction.java @@ -42,6 +42,12 @@ public static class Request extends ActionRequest { public Request() { } + public Request(StreamInput in) throws IOException { + super(in); + jobId = in.readString(); + snapshotId = in.readString(); + } + public Request(String jobId, String snapshotId) { this.jobId = ExceptionsHelper.requireNonNull(jobId, Job.ID.getPreferredName()); this.snapshotId = ExceptionsHelper.requireNonNull(snapshotId, ModelSnapshotField.SNAPSHOT_ID.getPreferredName()); @@ -62,9 +68,7 @@ public ActionRequestValidationException validate() { @Override public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - jobId = in.readString(); - snapshotId = in.readString(); + throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java index 6a33fa1fdbd6d..565132582e71e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java @@ -61,6 +61,11 @@ public Request(Detector detector) { this.detector = detector; } + public Request(StreamInput in) throws IOException { + super(in); + detector = new Detector(in); + } + public Detector getDetector() { return detector; } @@ -78,8 +83,7 @@ public void writeTo(StreamOutput out) throws IOException { @Override public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - detector = new Detector(in); + throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java index f6abae83eef2d..473ea57505587 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java @@ -78,6 +78,11 @@ public Request(Job job) { this.job = job; } + public Request(StreamInput in) throws IOException { + super(in); + job = new Job(in); + } + public Job getJob() { return job; } @@ -95,8 +100,7 @@ public void writeTo(StreamOutput out) throws IOException { @Override public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - job = new Job(in); + throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); } @Override diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteCalendarAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteCalendarAction.java index 918637a7c4936..1205590a8095d 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteCalendarAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteCalendarAction.java @@ -38,7 +38,7 @@ public class TransportDeleteCalendarAction extends HandledTransportAction Date: Tue, 16 Jul 2019 15:57:26 -0700 Subject: [PATCH 2/5] fix line length --- .../xpack/core/action/AbstractTransportGetResourcesAction.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/AbstractTransportGetResourcesAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/AbstractTransportGetResourcesAction.java index 003b702020721..41e2605d9dba3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/AbstractTransportGetResourcesAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/AbstractTransportGetResourcesAction.java @@ -61,7 +61,8 @@ public abstract class AbstractTransportGetResourcesAction request, Client client, NamedXContentRegistry xContentRegistry) { + Writeable.Reader request, Client client, + NamedXContentRegistry xContentRegistry) { super(actionName, transportService, actionFilters, request); this.client = Objects.requireNonNull(client); this.xContentRegistry = Objects.requireNonNull(xContentRegistry); From 128b7def9a0ce9eef86e751884fd280a02579fde Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 16 Jul 2019 17:40:24 -0700 Subject: [PATCH 3/5] fix streaming test --- .../ml/action/ValidateJobConfigActionRequestTests.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigActionRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigActionRequestTests.java index ac2d559c29aa1..f9226b336079d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigActionRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigActionRequestTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.core.ml.action; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; @@ -13,7 +14,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.test.AbstractStreamableTestCase; +import org.elasticsearch.test.AbstractWireSerializingTestCase; import org.elasticsearch.xpack.core.ml.action.ValidateJobConfigAction.Request; import org.elasticsearch.xpack.core.ml.job.config.Job; @@ -23,7 +24,7 @@ import static org.elasticsearch.xpack.core.ml.job.config.JobTests.buildJobBuilder; import static org.elasticsearch.xpack.core.ml.job.config.JobTests.randomValidJobId; -public class ValidateJobConfigActionRequestTests extends AbstractStreamableTestCase { +public class ValidateJobConfigActionRequestTests extends AbstractWireSerializingTestCase { @Override protected Request createTestInstance() { @@ -31,8 +32,8 @@ protected Request createTestInstance() { } @Override - protected Request createBlankInstance() { - return new Request(); + protected Writeable.Reader instanceReader() { + return Request::new; } public void testParseRequest_InvalidCreateSetting() throws IOException { From 678866fcb76100ca318a7a2d10214f8c1cfcecc9 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Wed, 17 Jul 2019 11:14:25 -0700 Subject: [PATCH 4/5] fix tests --- .../core/ml/action/ValidateDetectorAction.java | 7 +------ .../action/ValidateDetectorActionRequestTests.java | 13 +++++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java index 565132582e71e..f7bcecc332177 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java @@ -28,12 +28,7 @@ public class ValidateDetectorAction extends ActionType { public static final String NAME = "cluster:admin/xpack/ml/job/validate/detector"; protected ValidateDetectorAction() { - super(NAME); - } - - @Override - public Writeable.Reader getResponseReader() { - return AcknowledgedResponse::new; + super(NAME, AcknowledgedResponse::new); } public static class RequestBuilder extends ActionRequestBuilder { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorActionRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorActionRequestTests.java index d49908b1f1bae..993ed466777f5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorActionRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorActionRequestTests.java @@ -5,12 +5,13 @@ */ package org.elasticsearch.xpack.core.ml.action; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.test.AbstractStreamableXContentTestCase; +import org.elasticsearch.test.AbstractSerializingTestCase; import org.elasticsearch.xpack.core.ml.action.ValidateDetectorAction.Request; import org.elasticsearch.xpack.core.ml.job.config.Detector; -public class ValidateDetectorActionRequestTests extends AbstractStreamableXContentTestCase { +public class ValidateDetectorActionRequestTests extends AbstractSerializingTestCase { @Override protected Request createTestInstance() { @@ -24,13 +25,13 @@ protected Request createTestInstance() { } @Override - protected boolean supportsUnknownFields() { - return false; + protected Writeable.Reader instanceReader() { + return Request::new; } @Override - protected Request createBlankInstance() { - return new Request(); + protected boolean supportsUnknownFields() { + return false; } @Override From f464291e9f5cd129717baad8b028cdfe26992618 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Wed, 17 Jul 2019 14:27:15 -0700 Subject: [PATCH 5/5] fix checkstyle --- .../xpack/core/ml/action/ValidateDetectorAction.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java index f7bcecc332177..d584bd18dc716 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java @@ -13,7 +13,6 @@ import org.elasticsearch.client.ElasticsearchClient; 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.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser;