Skip to content

Remove AcknowledgedRestListener in favour of RestToXContentListener #28724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 22, 2018
Merged

Remove AcknowledgedRestListener in favour of RestToXContentListener #28724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ccfd123
Remove AcknowledgedRestListener in favour of RestToXContentListener
javanna Feb 16, 2018
7c89a40
Merge branch 'master' into enhancement/remove_ack_rest_listener
javanna Feb 19, 2018
718ceb4
Merge branch 'master' into enhancement/remove_ack_rest_listener
javanna Feb 19, 2018
94724b1
Merge branch 'master' into enhancement/remove_ack_rest_listener
javanna Feb 21, 2018
c899ee0
adjust ShardsAcknowledgedResponse xcontent output
javanna Feb 21, 2018
4fc3f8b
add test
javanna Feb 21, 2018
6ad82d0
Merge branch 'master' into enhancement/remove_ack_rest_listener
javanna Feb 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...s/ingest-common/src/main/java/org/elasticsearch/ingest/common/GrokProcessorGetAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestBuilder;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.HandledTransportAction;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.client.ElasticsearchClient;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
Expand Down Expand Up @@ -85,7 +85,7 @@ public RequestBuilder(ElasticsearchClient client) {
}
}

public static class Response extends AcknowledgedResponse implements ToXContentObject {
public static class Response extends ActionResponse implements ToXContentObject {
private Map<String, String> grokPatterns;

public Response(Map<String, String> grokPatterns) {
Expand Down
20 changes: 7 additions & 13 deletions modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/IngestCommonPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,6 @@

package org.elasticsearch.ingest.common;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
Expand All @@ -48,6 +35,13 @@
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestHandler;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

public class IngestCommonPlugin extends Plugin implements ActionPlugin, IngestPlugin {

static final Map<String, String> GROK_PATTERNS = Grok.getBuiltinPatterns();
Expand Down
15 changes: 14 additions & 1 deletion .../src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@
import org.elasticsearch.cluster.routing.allocation.RoutingExplanations;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;

/**
* Response returned after a cluster reroute request
*/
public class ClusterRerouteResponse extends AcknowledgedResponse {
public class ClusterRerouteResponse extends AcknowledgedResponse implements ToXContentObject {
Copy link
Member

@cbuescher cbuescher Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why this class didn't implement toXContent before. Was it not client-facing, that is not used in the rest API?

Copy link
Member Author

@javanna javanna Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the API is not supported yet by the high-level REST client


private ClusterState state;
private RoutingExplanations explanations;
Expand Down Expand Up @@ -71,4 +74,14 @@ public void writeTo(StreamOutput out) throws IOException {
writeAcknowledged(out);
RoutingExplanations.writeTo(explanations, out);
}

@Override
protected void addCustomFields(XContentBuilder builder, Params params) throws IOException {
builder.startObject("state");
state.toXContent(builder, params);
Copy link
Member

@cbuescher cbuescher Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see this was previously done in the RestClusterRerouteAction.

builder.endObject();
if (params.paramAsBoolean("explain", false)) {
explanations.toXContent(builder, ToXContent.EMPTY_PARAMS);
}
}
}
9 changes: 2 additions & 7 deletions ...n/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;

Expand All @@ -35,7 +34,7 @@
/**
* A response for a cluster update settings action.
*/
public class ClusterUpdateSettingsResponse extends AcknowledgedResponse implements ToXContentObject {
public class ClusterUpdateSettingsResponse extends AcknowledgedResponse {

private static final ParseField PERSISTENT = new ParseField("persistent");
private static final ParseField TRANSIENT = new ParseField("transient");
Expand Down Expand Up @@ -91,17 +90,13 @@ public void writeTo(StreamOutput out) throws IOException {
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
addAcknowledgedField(builder);
protected void addCustomFields(XContentBuilder builder, Params params) throws IOException {
builder.startObject(PERSISTENT.getPreferredName());
persistentSettings.toXContent(builder, params);
builder.endObject();
builder.startObject(TRANSIENT.getPreferredName());
transientSettings.toXContent(builder, params);
builder.endObject();
builder.endObject();
return builder;
}

public static ClusterUpdateSettingsResponse fromXContent(XContentParser parser) {
Expand Down
12 changes: 1 addition & 11 deletions ...er/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,14 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
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;

/**
* A response for a add/remove alias action.
*/
public class IndicesAliasesResponse extends AcknowledgedResponse implements ToXContentObject {
public class IndicesAliasesResponse extends AcknowledgedResponse {

private static final ConstructingObjectParser<IndicesAliasesResponse, Void> PARSER = new ConstructingObjectParser<>("indices_aliases",
true, args -> new IndicesAliasesResponse((boolean) args[0]));
Expand All @@ -59,14 +57,6 @@ public void writeTo(StreamOutput out) throws IOException {
writeAcknowledged(out);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
addAcknowledgedField(builder);
builder.endObject();
return builder;
}

public static IndicesAliasesResponse fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}
Expand Down
12 changes: 1 addition & 11 deletions server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,14 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
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;

/**
* A response for a close index action.
*/
public class CloseIndexResponse extends AcknowledgedResponse implements ToXContentObject {
public class CloseIndexResponse extends AcknowledgedResponse {
private static final ConstructingObjectParser<CloseIndexResponse, Void> PARSER = new ConstructingObjectParser<>("close_index", true,
args -> new CloseIndexResponse((boolean) args[0]));

Expand All @@ -59,14 +57,6 @@ public void writeTo(StreamOutput out) throws IOException {
writeAcknowledged(out);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
addAcknowledgedField(builder);
builder.endObject();
return builder;
}

public static CloseIndexResponse fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}
Expand Down
11 changes: 3 additions & 8 deletions server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;

Expand All @@ -38,7 +37,7 @@
/**
* A response for a create index action.
*/
public class CreateIndexResponse extends ShardsAcknowledgedResponse implements ToXContentObject {
public class CreateIndexResponse extends ShardsAcknowledgedResponse {

private static final ParseField INDEX = new ParseField("index");

Expand Down Expand Up @@ -89,13 +88,9 @@ public String index() {
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
addAcknowledgedField(builder);
addShardsAcknowledgedField(builder);
protected void addCustomFields(XContentBuilder builder, Params params) throws IOException {
super.addCustomFields(builder, params);
builder.field(INDEX.getPreferredName(), index());
builder.endObject();
return builder;
}

public static CreateIndexResponse fromXContent(XContentParser parser) {
Expand Down
11 changes: 1 addition & 10 deletions server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;

Expand All @@ -32,7 +31,7 @@
/**
* A response for a delete index action.
*/
public class DeleteIndexResponse extends AcknowledgedResponse implements ToXContentObject {
public class DeleteIndexResponse extends AcknowledgedResponse {

private static final ConstructingObjectParser<DeleteIndexResponse, Void> PARSER = new ConstructingObjectParser<>("delete_index",
true, args -> new DeleteIndexResponse((boolean) args[0]));
Expand Down Expand Up @@ -60,14 +59,6 @@ public void writeTo(StreamOutput out) throws IOException {
writeAcknowledged(out);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
addAcknowledgedField(builder);
builder.endObject();
return builder;
}

public static DeleteIndexResponse fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}
Expand Down
12 changes: 1 addition & 11 deletions .../src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,14 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
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;

/**
* The response of put mapping operation.
*/
public class PutMappingResponse extends AcknowledgedResponse implements ToXContentObject {
public class PutMappingResponse extends AcknowledgedResponse {

private static final ConstructingObjectParser<PutMappingResponse, Void> PARSER = new ConstructingObjectParser<>("put_mapping",
true, args -> new PutMappingResponse((boolean) args[0]));
Expand Down Expand Up @@ -61,14 +59,6 @@ public void writeTo(StreamOutput out) throws IOException {
writeAcknowledged(out);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
addAcknowledgedField(builder);
builder.endObject();
return builder;
}

public static PutMappingResponse fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}
Expand Down
13 changes: 2 additions & 11 deletions server/src/main/java/org/elasticsearch/action/admin/indices/open/OpenIndexResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@

import org.elasticsearch.Version;
import org.elasticsearch.action.support.master.ShardsAcknowledgedResponse;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;

Expand All @@ -33,7 +33,7 @@
/**
* A response for a open index action.
*/
public class OpenIndexResponse extends ShardsAcknowledgedResponse implements ToXContentObject {
public class OpenIndexResponse extends ShardsAcknowledgedResponse {

private static final ConstructingObjectParser<OpenIndexResponse, Void> PARSER = new ConstructingObjectParser<>("open_index", true,
args -> new OpenIndexResponse((boolean) args[0], (boolean) args[1]));
Expand Down Expand Up @@ -67,15 +67,6 @@ public void writeTo(StreamOutput out) throws IOException {
}
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
addAcknowledgedField(builder);
addShardsAcknowledgedField(builder);
builder.endObject();
return builder;
}

public static OpenIndexResponse fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}
Expand Down
8 changes: 2 additions & 6 deletions server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,21 +145,17 @@ public void writeTo(StreamOutput out) throws IOException {
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
protected void addCustomFields(XContentBuilder builder, Params params) throws IOException {
super.addCustomFields(builder, params);
builder.field(OLD_INDEX.getPreferredName(), oldIndex);
builder.field(NEW_INDEX.getPreferredName(), newIndex);
builder.field(ROLLED_OVER.getPreferredName(), rolledOver);
builder.field(DRY_RUN.getPreferredName(), dryRun);
addAcknowledgedField(builder);
addShardsAcknowledgedField(builder);
builder.startObject(CONDITIONS.getPreferredName());
for (Map.Entry<String, Boolean> entry : conditionStatus.entrySet()) {
builder.field(entry.getKey(), entry.getValue());
}
builder.endObject();
builder.endObject();
return builder;
}

public static RolloverResponse fromXContent(XContentParser parser) {
Expand Down
14 changes: 12 additions & 2 deletions server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;
Expand All @@ -35,7 +36,7 @@
* Abstract class that allows to mark action responses that support acknowledgements.
* Facilitates consistency across different api.
*/
public abstract class AcknowledgedResponse extends ActionResponse {
public abstract class AcknowledgedResponse extends ActionResponse implements ToXContentObject {

private static final ParseField ACKNOWLEDGED = new ParseField("acknowledged");

Expand Down Expand Up @@ -76,8 +77,17 @@ protected void writeAcknowledged(StreamOutput out) throws IOException {
out.writeBoolean(acknowledged);
}

protected void addAcknowledgedField(XContentBuilder builder) throws IOException {
@Override
public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(ACKNOWLEDGED.getPreferredName(), isAcknowledged());
addCustomFields(builder, params);
builder.endObject();
return builder;
}

protected void addCustomFields(XContentBuilder builder, Params params) throws IOException {
Copy link
Member

@cbuescher cbuescher Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if this could be abstract, so subclasses don't forget to implement it. Looks like many would have an empty implementation though, so just a thought.

Copy link
Member Author

@javanna javanna Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be, but the classes that have a non-empty impl for it are more of an exception, so maybe it's ok like this?


}

@Override
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/org/elasticsearch/action/support/master/ShardsAcknowledgedResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ protected void writeShardsAcknowledged(StreamOutput out) throws IOException {
out.writeBoolean(shardsAcknowledged);
}

protected void addShardsAcknowledgedField(XContentBuilder builder) throws IOException {
@Override
protected void addCustomFields(XContentBuilder builder, Params params) throws IOException {
builder.field(SHARDS_ACKNOWLEDGED.getPreferredName(), isShardsAcknowledged());
}

Expand Down
Loading