Skip to content

Commit

Permalink
Remove StatusToXContent interface (#99824)
Browse files Browse the repository at this point in the history
The StatusToXContent interface extends ToXContent with a REST status
method. It is used by RestStatusToXContentListener, which uses the extra
method to set the status of the RestResponse. However, given the
generics parametrization of RestToXContentListener, we can already get
the status of a response object without having these extra interfaces,
which mix serialization and status concerns.

This commit removes StatusToXContent and RestStatusToXContentListener,
instead adding some extra optional parameters to RestToXContentListener to
handle retrieving status and location information from the passed in Response
object.
  • Loading branch information
romseygeek authored Oct 2, 2023
1 parent e453227 commit f580501
Show file tree
Hide file tree
Showing 78 changed files with 244 additions and 459 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.Scope;
import org.elasticsearch.rest.ServerlessScope;
import org.elasticsearch.rest.action.RestStatusToXContentListener;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.rest.action.search.RestSearchAction;
import org.elasticsearch.xcontent.XContentParser;

Expand Down Expand Up @@ -76,7 +76,11 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
if (searchRequest.source().explain() != null) {
searchTemplateRequest.setExplain(searchRequest.source().explain());
}
return channel -> client.execute(SearchTemplateAction.INSTANCE, searchTemplateRequest, new RestStatusToXContentListener<>(channel));
return channel -> client.execute(
SearchTemplateAction.INSTANCE,
searchTemplateRequest,
new RestToXContentListener<>(channel, SearchTemplateResponse::status)
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ChunkedToXContent;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentFactory;
import org.elasticsearch.xcontent.XContentParser;
Expand All @@ -26,7 +26,7 @@
import java.io.InputStream;
import java.util.Map;

public class SearchTemplateResponse extends ActionResponse implements StatusToXContentObject {
public class SearchTemplateResponse extends ActionResponse implements ToXContentObject {
public static ParseField TEMPLATE_OUTPUT_FIELD = new ParseField("template_output");

/** Contains the source of the rendered template **/
Expand Down Expand Up @@ -113,7 +113,6 @@ void innerToXContent(XContentBuilder builder, Params params) throws IOException
}
}

@Override
public RestStatus status() {
if (hasResponse()) {
return response.status();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.elasticsearch.http;

import org.elasticsearch.Version;
import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.client.Request;
Expand All @@ -32,7 +33,7 @@
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestHandler;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestStatusToXContentListener;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;
Expand Down Expand Up @@ -234,7 +235,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
indexRequest.source(Map.of("some_field", "some_value"));
return channel -> client.index(
indexRequest,
new RestStatusToXContentListener<>(channel, r -> r.getLocation(indexRequest.routing()))
new RestToXContentListener<>(channel, DocWriteResponse::status, r -> r.getLocation(indexRequest.routing()))
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
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.StatusToXContentObject;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.index.Index;
Expand All @@ -25,6 +24,7 @@
import org.elasticsearch.index.seqno.SequenceNumbers;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

Expand All @@ -41,7 +41,7 @@
/**
* A base class for the response of a write operation that involves a single doc
*/
public abstract class DocWriteResponse extends ReplicationResponse implements WriteResponse, StatusToXContentObject {
public abstract class DocWriteResponse extends ReplicationResponse implements WriteResponse, ToXContentObject {

private static final String _SHARDS = "_shards";
private static final String _INDEX = "_index";
Expand Down Expand Up @@ -216,7 +216,6 @@ public void setForcedRefresh(boolean forcedRefresh) {
}

/** returns the rest status for this response (based on {@link ShardInfo#status()} */
@Override
public RestStatus status() {
return getShardInfo().status();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

Expand All @@ -37,7 +37,7 @@
import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;

public class ClusterHealthResponse extends ActionResponse implements StatusToXContentObject {
public class ClusterHealthResponse extends ActionResponse implements ToXContentObject {
private static final String CLUSTER_NAME = "cluster_name";
private static final String STATUS = "status";
private static final String TIMED_OUT = "timed_out";
Expand Down Expand Up @@ -333,7 +333,6 @@ public String toString() {
return Strings.toString(this);
}

@Override
public RestStatus status() {
return isTimedOut() ? RestStatus.REQUEST_TIMEOUT : RestStatus.OK;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.script.ScriptContextInfo;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

Expand All @@ -30,7 +29,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;

public class GetScriptContextResponse extends ActionResponse implements StatusToXContentObject {
public class GetScriptContextResponse extends ActionResponse implements ToXContentObject {

private static final ParseField CONTEXTS = new ParseField("contexts");
final Map<String, ScriptContextInfo> contexts;
Expand Down Expand Up @@ -87,11 +86,6 @@ public void writeTo(StreamOutput out) throws IOException {
}
}

@Override
public RestStatus status() {
return RestStatus.OK;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject().startArray(CONTEXTS.getPreferredName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,15 @@
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.StatusToXContentObject;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.script.ScriptLanguagesInfo;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

import java.io.IOException;
import java.util.Objects;

public class GetScriptLanguageResponse extends ActionResponse implements StatusToXContentObject, Writeable {
public class GetScriptLanguageResponse extends ActionResponse implements ToXContentObject, Writeable {
public final ScriptLanguagesInfo info;

GetScriptLanguageResponse(ScriptLanguagesInfo info) {
Expand All @@ -38,11 +37,6 @@ public void writeTo(StreamOutput out) throws IOException {
info.writeTo(out);
}

@Override
public RestStatus status() {
return RestStatus.OK;
}

public static GetScriptLanguageResponse fromXContent(XContentParser parser) throws IOException {
return new GetScriptLanguageResponse(ScriptLanguagesInfo.fromXContent(parser));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.script.StoredScriptSource;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

Expand All @@ -26,7 +26,7 @@
import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;

public class GetStoredScriptResponse extends ActionResponse implements StatusToXContentObject {
public class GetStoredScriptResponse extends ActionResponse implements ToXContentObject {

public static final ParseField _ID_PARSE_FIELD = new ParseField("_id");
public static final ParseField FOUND_PARSE_FIELD = new ParseField("found");
Expand Down Expand Up @@ -84,7 +84,6 @@ public StoredScriptSource getSource() {
return source;
}

@Override
public RestStatus status() {
return source != null ? RestStatus.OK : RestStatus.NOT_FOUND;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContent;
import org.elasticsearch.xcontent.XContentBuilder;

Expand All @@ -34,7 +34,7 @@
* information for each dangling index is presented under the "dangling_indices" key. If any nodes
* in the cluster failed to answer, the details are presented under the "_nodes.failures" key.
*/
public class ListDanglingIndicesResponse extends BaseNodesResponse<NodeListDanglingIndicesResponse> implements StatusToXContentObject {
public class ListDanglingIndicesResponse extends BaseNodesResponse<NodeListDanglingIndicesResponse> implements ToXContentObject {

public ListDanglingIndicesResponse(StreamInput in) throws IOException {
super(in);
Expand All @@ -48,7 +48,6 @@ public ListDanglingIndicesResponse(
super(clusterName, nodes, failures);
}

@Override
public RestStatus status() {
return this.hasFailures() ? RestStatus.INTERNAL_SERVER_ERROR : RestStatus.OK;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
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.StatusToXContentObject;
import org.elasticsearch.core.CheckedConsumer;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.index.mapper.MapperService;
Expand All @@ -30,6 +29,7 @@
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentFragment;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

Expand All @@ -44,14 +44,13 @@
* Represents a single item response for an action executed as part of the bulk API. Holds the index/type/id
* of the relevant action, and if it has failed or not (with the failure message in case it failed).
*/
public class BulkItemResponse implements Writeable, StatusToXContentObject {
public class BulkItemResponse implements Writeable, ToXContentObject {

private static final String _INDEX = "_index";
private static final String _ID = "_id";
private static final String STATUS = "status";
private static final String ERROR = "error";

@Override
public RestStatus status() {
return failure == null ? response.status() : failure.getStatus();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.index.get.GetResult;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

Expand All @@ -33,7 +33,7 @@
/**
* Response containing the score explanation.
*/
public class ExplainResponse extends ActionResponse implements StatusToXContentObject {
public class ExplainResponse extends ActionResponse implements ToXContentObject {

private static final ParseField _INDEX = new ParseField("_index");
private static final ParseField _ID = new ParseField("_id");
Expand All @@ -44,9 +44,9 @@ public class ExplainResponse extends ActionResponse implements StatusToXContentO
private static final ParseField DETAILS = new ParseField("details");
private static final ParseField GET = new ParseField("get");

private String index;
private String id;
private boolean exists;
private final String index;
private final String id;
private final boolean exists;
private Explanation explanation;
private GetResult getResult;

Expand Down Expand Up @@ -110,7 +110,6 @@ public GetResult getGetResult() {
return getResult;
}

@Override
public RestStatus status() {
return exists ? RestStatus.OK : RestStatus.NOT_FOUND;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.ingest.PipelineConfiguration;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentParser.Token;
Expand All @@ -29,9 +29,9 @@

import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;

public class GetPipelineResponse extends ActionResponse implements StatusToXContentObject {
public class GetPipelineResponse extends ActionResponse implements ToXContentObject {

private List<PipelineConfiguration> pipelines;
private final List<PipelineConfiguration> pipelines;
private final boolean summary;

public GetPipelineResponse(StreamInput in) throws IOException {
Expand Down Expand Up @@ -76,7 +76,6 @@ public boolean isSummary() {
return summary;
}

@Override
public RestStatus status() {
return isFound() ? RestStatus.OK : RestStatus.NOT_FOUND;
}
Expand Down
Loading

0 comments on commit f580501

Please sign in to comment.