Skip to content

Commit

Permalink
migrate more ML actions off of using Request suppliers (#44462) (#44529)
Browse files Browse the repository at this point in the history
many classes still use the Streamable constructors of HandledTransportAction,
this commit moves more of those classes to the new Writeable constructors.

relates #34389.
  • Loading branch information
talevy committed Jul 18, 2019
1 parent 075a3f0 commit a5ad594
Show file tree
Hide file tree
Showing 15 changed files with 46 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -62,8 +61,9 @@ public abstract class AbstractTransportGetResourcesAction<Resource extends ToXCo
private final NamedXContentRegistry xContentRegistry;

protected AbstractTransportGetResourcesAction(String actionName, TransportService transportService, ActionFilters actionFilters,
Supplier<Request> request, Client client, NamedXContentRegistry xContentRegistry) {
super(actionName, transportService, request, actionFilters);
Writeable.Reader<Request> request, Client client,
NamedXContentRegistry xContentRegistry) {
super(actionName, transportService, actionFilters, request);
this.client = Objects.requireNonNull(client);
this.xContentRegistry = Objects.requireNonNull(xContentRegistry);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,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());
Expand All @@ -56,9 +62,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,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;
}
Expand All @@ -72,8 +77,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,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;
}
Expand All @@ -89,8 +94,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Request> {
public class ValidateDetectorActionRequestTests extends AbstractSerializingTestCase<Request> {

@Override
protected Request createTestInstance() {
Expand All @@ -24,13 +25,13 @@ protected Request createTestInstance() {
}

@Override
protected boolean supportsUnknownFields() {
return false;
protected Writeable.Reader<Request> instanceReader() {
return Request::new;
}

@Override
protected Request createBlankInstance() {
return new Request();
protected boolean supportsUnknownFields() {
return false;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
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;
import org.elasticsearch.common.xcontent.XContentBuilder;
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;

Expand All @@ -23,16 +24,16 @@
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<Request> {
public class ValidateJobConfigActionRequestTests extends AbstractWireSerializingTestCase<Request> {

@Override
protected Request createTestInstance() {
return new Request(buildJobBuilder(randomValidJobId(), new Date()).build());
}

@Override
protected Request createBlankInstance() {
return new Request();
protected Writeable.Reader<Request> instanceReader() {
return Request::new;
}

public void testParseRequest_InvalidCreateSetting() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class TransportDeleteCalendarAction extends HandledTransportAction<Delete
public TransportDeleteCalendarAction(TransportService transportService,
ActionFilters actionFilters, Client client, JobManager jobManager,
JobResultsProvider jobResultsProvider) {
super(DeleteCalendarAction.NAME, transportService, DeleteCalendarAction.Request::new, actionFilters);
super(DeleteCalendarAction.NAME, transportService, actionFilters, DeleteCalendarAction.Request::new);
this.client = client;
this.jobManager = jobManager;
this.jobResultsProvider = jobResultsProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class TransportDeleteCalendarEventAction extends HandledTransportAction<D
@Inject
public TransportDeleteCalendarEventAction(TransportService transportService, ActionFilters actionFilters,
Client client, JobResultsProvider jobResultsProvider, JobManager jobManager) {
super(DeleteCalendarEventAction.NAME, transportService, DeleteCalendarEventAction.Request::new, actionFilters);
super(DeleteCalendarEventAction.NAME, transportService, actionFilters, DeleteCalendarEventAction.Request::new);
this.client = client;
this.jobResultsProvider = jobResultsProvider;
this.jobManager = jobManager;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class TransportDeleteExpiredDataAction extends HandledTransportAction<Del
@Inject
public TransportDeleteExpiredDataAction(ThreadPool threadPool, TransportService transportService,
ActionFilters actionFilters, Client client, ClusterService clusterService) {
super(DeleteExpiredDataAction.NAME, transportService, DeleteExpiredDataAction.Request::new, actionFilters);
super(DeleteExpiredDataAction.NAME, transportService, actionFilters, DeleteExpiredDataAction.Request::new);
this.threadPool = threadPool;
this.client = ClientHelper.clientWithOrigin(client, ClientHelper.ML_ORIGIN);
this.clusterService = clusterService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class TransportDeleteFilterAction extends HandledTransportAction<DeleteFi
public TransportDeleteFilterAction(TransportService transportService,
ActionFilters actionFilters, Client client,
JobConfigProvider jobConfigProvider) {
super(DeleteFilterAction.NAME, transportService, DeleteFilterAction.Request::new, actionFilters);
super(DeleteFilterAction.NAME, transportService, actionFilters, DeleteFilterAction.Request::new);
this.client = client;
this.jobConfigProvider = jobConfigProvider;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class TransportDeleteModelSnapshotAction extends HandledTransportAction<D
public TransportDeleteModelSnapshotAction(TransportService transportService, ActionFilters actionFilters,
JobResultsProvider jobResultsProvider, Client client, JobManager jobManager,
Auditor auditor) {
super(DeleteModelSnapshotAction.NAME, transportService, DeleteModelSnapshotAction.Request::new, actionFilters);
super(DeleteModelSnapshotAction.NAME, transportService, actionFilters, DeleteModelSnapshotAction.Request::new);
this.client = client;
this.jobManager = jobManager;
this.jobResultsProvider = jobResultsProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ public class TransportGetCalendarEventsAction extends HandledTransportAction<Get
public TransportGetCalendarEventsAction(TransportService transportService,
ActionFilters actionFilters, JobResultsProvider jobResultsProvider,
JobConfigProvider jobConfigProvider) {
super(GetCalendarEventsAction.NAME, transportService, GetCalendarEventsAction.Request::new, actionFilters
);
super(GetCalendarEventsAction.NAME, transportService, actionFilters, GetCalendarEventsAction.Request::new);
this.jobResultsProvider = jobResultsProvider;
this.jobConfigProvider = jobConfigProvider;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class TransportValidateDetectorAction extends HandledTransportAction<Vali

@Inject
public TransportValidateDetectorAction(TransportService transportService, ActionFilters actionFilters) {
super(ValidateDetectorAction.NAME, transportService, ValidateDetectorAction.Request::new, actionFilters);
super(ValidateDetectorAction.NAME, transportService, actionFilters, ValidateDetectorAction.Request::new);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ public class TransportValidateJobConfigAction extends HandledTransportAction<Val

@Inject
public TransportValidateJobConfigAction(TransportService transportService, ActionFilters actionFilters) {
super(ValidateJobConfigAction.NAME, transportService, ValidateJobConfigAction.Request::new, actionFilters
);
super(ValidateJobConfigAction.NAME, transportService, actionFilters, ValidateJobConfigAction.Request::new);
}

@Override
Expand Down

0 comments on commit a5ad594

Please sign in to comment.