Skip to content

Commit 377274d

Browse files
PnPiejavanna
PnPie
authored and
javanna
committed
Change BroadcastResponse from ToXContentFragment to ToXContentObject (#28878)
While working on #27799, we find that it might make sense to change BroadcastResponse from ToXContentFragment to ToXContentObject, seeing that it's rather a complete XContent object and also the other Responses are normally ToXContentObject. By doing this, we can also move the XContent build logic of BroadcastResponse's subclasses, from Rest Layer to the concrete classes themselves. Relates to #3889
1 parent 60dfa0d commit 377274d

20 files changed

+105
-243
lines changed

‎server/src/main/java/org/elasticsearch/action/admin/indices/recovery/RecoveryResponse.java‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.common.Strings;
2525
import org.elasticsearch.common.io.stream.StreamInput;
2626
import org.elasticsearch.common.io.stream.StreamOutput;
27-
import org.elasticsearch.common.xcontent.ToXContentFragment;
2827
import org.elasticsearch.common.xcontent.XContentBuilder;
2928
import org.elasticsearch.indices.recovery.RecoveryState;
3029

@@ -37,9 +36,8 @@
3736
/**
3837
* Information regarding the recovery state of indices and their associated shards.
3938
*/
40-
public class RecoveryResponse extends BroadcastResponse implements ToXContentFragment {
39+
public class RecoveryResponse extends BroadcastResponse {
4140

42-
private boolean detailed = false;
4341
private Map<String, List<RecoveryState>> shardRecoveryStates = new HashMap<>();
4442

4543
public RecoveryResponse() { }
@@ -51,36 +49,26 @@ public RecoveryResponse() { }
5149
* @param totalShards Total count of shards seen
5250
* @param successfulShards Count of shards successfully processed
5351
* @param failedShards Count of shards which failed to process
54-
* @param detailed Display detailed metrics
5552
* @param shardRecoveryStates Map of indices to shard recovery information
5653
* @param shardFailures List of failures processing shards
5754
*/
58-
public RecoveryResponse(int totalShards, int successfulShards, int failedShards, boolean detailed,
59-
Map<String, List<RecoveryState>> shardRecoveryStates,
55+
public RecoveryResponse(int totalShards, int successfulShards, int failedShards, Map<String, List<RecoveryState>> shardRecoveryStates,
6056
List<DefaultShardOperationFailedException> shardFailures) {
6157
super(totalShards, successfulShards, failedShards, shardFailures);
6258
this.shardRecoveryStates = shardRecoveryStates;
63-
this.detailed = detailed;
6459
}
6560

6661
public boolean hasRecoveries() {
6762
return shardRecoveryStates.size() > 0;
6863
}
6964

70-
public boolean detailed() {
71-
return detailed;
72-
}
73-
74-
public void detailed(boolean detailed) {
75-
this.detailed = detailed;
76-
}
77-
7865
public Map<String, List<RecoveryState>> shardRecoveryStates() {
7966
return shardRecoveryStates;
8067
}
8168

8269
@Override
8370
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
71+
builder.startObject();
8472
if (hasRecoveries()) {
8573
for (String index : shardRecoveryStates.keySet()) {
8674
List<RecoveryState> recoveryStates = shardRecoveryStates.get(index);
@@ -98,6 +86,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
9886
builder.endObject();
9987
}
10088
}
89+
builder.endObject();
10190
return builder;
10291
}
10392

@@ -133,4 +122,4 @@ public void readFrom(StreamInput in) throws IOException {
133122
public String toString() {
134123
return Strings.toString(this, true, true);
135124
}
136-
}
125+
}

‎server/src/main/java/org/elasticsearch/action/admin/indices/recovery/TransportRecoveryAction.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ protected RecoveryResponse newResponse(RecoveryRequest request, int totalShards,
8787
shardResponses.get(indexName).add(recoveryState);
8888
}
8989
}
90-
return new RecoveryResponse(totalShards, successfulShards, failedShards, request.detailed(), shardResponses, shardFailures);
90+
return new RecoveryResponse(totalShards, successfulShards, failedShards, shardResponses, shardFailures);
9191
}
9292

9393
@Override
@@ -118,4 +118,4 @@ protected ClusterBlockException checkGlobalBlock(ClusterState state, RecoveryReq
118118
protected ClusterBlockException checkRequestBlock(ClusterState state, RecoveryRequest request, String[] concreteIndices) {
119119
return state.blocks().indicesBlockedException(ClusterBlockLevel.READ, concreteIndices);
120120
}
121-
}
121+
}

‎server/src/main/java/org/elasticsearch/action/admin/indices/segments/IndicesSegmentResponse.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.elasticsearch.common.io.stream.StreamInput;
3030
import org.elasticsearch.common.io.stream.StreamOutput;
3131
import org.elasticsearch.common.unit.ByteSizeValue;
32-
import org.elasticsearch.common.xcontent.ToXContentFragment;
3332
import org.elasticsearch.common.xcontent.XContentBuilder;
3433
import org.elasticsearch.index.engine.Segment;
3534

@@ -43,7 +42,7 @@
4342
import java.util.Map;
4443
import java.util.Set;
4544

46-
public class IndicesSegmentResponse extends BroadcastResponse implements ToXContentFragment {
45+
public class IndicesSegmentResponse extends BroadcastResponse {
4746

4847
private ShardSegments[] shards;
4948

@@ -103,7 +102,7 @@ public void writeTo(StreamOutput out) throws IOException {
103102
}
104103

105104
@Override
106-
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
105+
protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
107106
builder.startObject(Fields.INDICES);
108107

109108
for (IndexSegments indexSegments : getIndices().values()) {
@@ -173,10 +172,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
173172
}
174173

175174
builder.endObject();
176-
return builder;
177175
}
178176

179-
static void toXContent(XContentBuilder builder, Sort sort) throws IOException {
177+
private static void toXContent(XContentBuilder builder, Sort sort) throws IOException {
180178
builder.startArray("sort");
181179
for (SortField field : sort.getSort()) {
182180
builder.startObject();
@@ -195,7 +193,7 @@ static void toXContent(XContentBuilder builder, Sort sort) throws IOException {
195193
builder.endArray();
196194
}
197195

198-
static void toXContent(XContentBuilder builder, Accountable tree) throws IOException {
196+
private static void toXContent(XContentBuilder builder, Accountable tree) throws IOException {
199197
builder.startObject();
200198
builder.field(Fields.DESCRIPTION, tree.toString());
201199
builder.humanReadableField(Fields.SIZE_IN_BYTES, Fields.SIZE, new ByteSizeValue(tree.ramBytesUsed()));

‎server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponse.java‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@
2525
import org.elasticsearch.common.Strings;
2626
import org.elasticsearch.common.io.stream.StreamInput;
2727
import org.elasticsearch.common.io.stream.StreamOutput;
28-
import org.elasticsearch.common.xcontent.ToXContentFragment;
2928
import org.elasticsearch.common.xcontent.XContentBuilder;
30-
import org.elasticsearch.common.xcontent.XContentFactory;
3129

3230
import java.io.IOException;
3331
import java.util.ArrayList;
@@ -39,7 +37,7 @@
3937

4038
import static java.util.Collections.unmodifiableMap;
4139

42-
public class IndicesStatsResponse extends BroadcastResponse implements ToXContentFragment {
40+
public class IndicesStatsResponse extends BroadcastResponse {
4341

4442
private ShardStats[] shards;
4543

@@ -147,15 +145,14 @@ public void writeTo(StreamOutput out) throws IOException {
147145
}
148146

149147
@Override
150-
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
148+
protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
151149
final String level = params.param("level", "indices");
152150
final boolean isLevelValid =
153151
"cluster".equalsIgnoreCase(level) || "indices".equalsIgnoreCase(level) || "shards".equalsIgnoreCase(level);
154152
if (!isLevelValid) {
155153
throw new IllegalArgumentException("level parameter must be one of [cluster] or [indices] or [shards] but was [" + level + "]");
156154
}
157155

158-
159156
builder.startObject("_all");
160157

161158
builder.startObject("primaries");
@@ -198,8 +195,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
198195
}
199196
builder.endObject();
200197
}
201-
202-
return builder;
203198
}
204199

205200
static final class Fields {
@@ -209,14 +204,6 @@ static final class Fields {
209204

210205
@Override
211206
public String toString() {
212-
try {
213-
XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint();
214-
builder.startObject();
215-
toXContent(builder, EMPTY_PARAMS);
216-
builder.endObject();
217-
return Strings.toString(builder);
218-
} catch (IOException e) {
219-
return "{ \"error\" : \"" + e.getMessage() + "\"}";
220-
}
207+
return Strings.toString(this, true, false);
221208
}
222209
}

‎server/src/main/java/org/elasticsearch/action/admin/indices/upgrade/get/UpgradeStatusResponse.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.elasticsearch.action.support.broadcast.BroadcastResponse;
2424
import org.elasticsearch.common.io.stream.StreamInput;
2525
import org.elasticsearch.common.io.stream.StreamOutput;
26-
import org.elasticsearch.common.xcontent.ToXContentFragment;
2726
import org.elasticsearch.common.xcontent.XContentBuilder;
2827

2928
import java.io.IOException;
@@ -34,7 +33,7 @@
3433
import java.util.Map;
3534
import java.util.Set;
3635

37-
public class UpgradeStatusResponse extends BroadcastResponse implements ToXContentFragment {
36+
public class UpgradeStatusResponse extends BroadcastResponse {
3837
private ShardUpgradeStatus[] shards;
3938

4039
private Map<String, IndexUpgradeStatus> indicesUpgradeStatus;
@@ -116,6 +115,7 @@ public long getToUpgradeBytesAncient() {
116115

117116
@Override
118117
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
118+
builder.startObject();
119119
builder.byteSizeField(Fields.SIZE_IN_BYTES, Fields.SIZE, getTotalBytes());
120120
builder.byteSizeField(Fields.SIZE_TO_UPGRADE_IN_BYTES, Fields.SIZE_TO_UPGRADE, getToUpgradeBytes());
121121
builder.byteSizeField(Fields.SIZE_TO_UPGRADE_ANCIENT_IN_BYTES, Fields.SIZE_TO_UPGRADE_ANCIENT, getToUpgradeBytesAncient());
@@ -161,6 +161,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
161161
}
162162
builder.endObject();
163163
}
164+
builder.endObject();
164165
return builder;
165166
}
166167

‎server/src/main/java/org/elasticsearch/action/admin/indices/upgrade/post/UpgradeResponse.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.common.collect.Tuple;
2626
import org.elasticsearch.common.io.stream.StreamInput;
2727
import org.elasticsearch.common.io.stream.StreamOutput;
28+
import org.elasticsearch.common.xcontent.XContentBuilder;
2829

2930
import java.io.IOException;
3031
import java.util.HashMap;
@@ -74,6 +75,18 @@ public void writeTo(StreamOutput out) throws IOException {
7475
}
7576
}
7677

78+
@Override
79+
protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
80+
builder.startObject("upgraded_indices");
81+
for (Map.Entry<String, Tuple<Version, String>> entry : versions.entrySet()) {
82+
builder.startObject(entry.getKey());
83+
builder.field("upgrade_version", entry.getValue().v1());
84+
builder.field("oldest_lucene_segment_version", entry.getValue().v2());
85+
builder.endObject();
86+
}
87+
builder.endObject();
88+
}
89+
7790
/**
7891
* Returns the highest upgrade version of the node that performed metadata upgrade and the
7992
* the version of the oldest lucene segment for each index that was upgraded.

‎server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/ValidateQueryResponse.java‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.elasticsearch.action.support.broadcast.BroadcastResponse;
2424
import org.elasticsearch.common.io.stream.StreamInput;
2525
import org.elasticsearch.common.io.stream.StreamOutput;
26+
import org.elasticsearch.common.xcontent.XContentBuilder;
2627

2728
import java.io.IOException;
2829
import java.util.ArrayList;
@@ -38,8 +39,15 @@
3839
*/
3940
public class ValidateQueryResponse extends BroadcastResponse {
4041

42+
public static final String INDEX_FIELD = "index";
43+
public static final String SHARD_FIELD = "shard";
44+
public static final String VALID_FIELD = "valid";
45+
public static final String EXPLANATIONS_FIELD = "explanations";
46+
public static final String ERROR_FIELD = "error";
47+
public static final String EXPLANATION_FIELD = "explanation";
48+
4149
private boolean valid;
42-
50+
4351
private List<QueryExplanation> queryExplanations;
4452

4553
ValidateQueryResponse() {
@@ -96,4 +104,30 @@ public void writeTo(StreamOutput out) throws IOException {
96104
}
97105

98106
}
107+
108+
@Override
109+
protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
110+
builder.field(VALID_FIELD, isValid());
111+
if (getQueryExplanation() != null && !getQueryExplanation().isEmpty()) {
112+
builder.startArray(EXPLANATIONS_FIELD);
113+
for (QueryExplanation explanation : getQueryExplanation()) {
114+
builder.startObject();
115+
if (explanation.getIndex() != null) {
116+
builder.field(INDEX_FIELD, explanation.getIndex());
117+
}
118+
if(explanation.getShard() >= 0) {
119+
builder.field(SHARD_FIELD, explanation.getShard());
120+
}
121+
builder.field(VALID_FIELD, explanation.isValid());
122+
if (explanation.getError() != null) {
123+
builder.field(ERROR_FIELD, explanation.getError());
124+
}
125+
if (explanation.getExplanation() != null) {
126+
builder.field(EXPLANATION_FIELD, explanation.getExplanation());
127+
}
128+
builder.endObject();
129+
}
130+
builder.endArray();
131+
}
132+
}
99133
}

‎server/src/main/java/org/elasticsearch/action/support/broadcast/BroadcastResponse.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import org.elasticsearch.common.io.stream.StreamInput;
2626
import org.elasticsearch.common.io.stream.StreamOutput;
2727
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
28-
import org.elasticsearch.common.xcontent.ToXContentFragment;
28+
import org.elasticsearch.common.xcontent.ToXContentObject;
2929
import org.elasticsearch.common.xcontent.XContentBuilder;
3030
import org.elasticsearch.rest.RestStatus;
3131
import org.elasticsearch.rest.action.RestActions;
@@ -40,7 +40,7 @@
4040
/**
4141
* Base class for all broadcast operation based responses.
4242
*/
43-
public class BroadcastResponse extends ActionResponse implements ToXContentFragment {
43+
public class BroadcastResponse extends ActionResponse implements ToXContentObject {
4444

4545
public static final DefaultShardOperationFailedException[] EMPTY = new DefaultShardOperationFailedException[0];
4646

@@ -149,7 +149,16 @@ public void writeTo(StreamOutput out) throws IOException {
149149

150150
@Override
151151
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
152+
builder.startObject();
152153
RestActions.buildBroadcastShardsHeader(builder, params, this);
154+
addCustomXContentFields(builder, params);
155+
builder.endObject();
153156
return builder;
154157
}
158+
159+
/**
160+
* Override in subclass to add custom fields following the common `_shards` field
161+
*/
162+
protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
163+
}
155164
}

‎server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestClearIndicesCacheAction.java‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,22 @@
2020
package org.elasticsearch.rest.action.admin.indices;
2121

2222
import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest;
23-
import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheResponse;
2423
import org.elasticsearch.action.support.IndicesOptions;
2524
import org.elasticsearch.client.node.NodeClient;
2625
import org.elasticsearch.common.ParseField;
2726
import org.elasticsearch.common.Strings;
2827
import org.elasticsearch.common.settings.Settings;
2928
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
30-
import org.elasticsearch.common.xcontent.XContentBuilder;
3129
import org.elasticsearch.rest.BaseRestHandler;
32-
import org.elasticsearch.rest.BytesRestResponse;
3330
import org.elasticsearch.rest.RestController;
3431
import org.elasticsearch.rest.RestRequest;
35-
import org.elasticsearch.rest.RestResponse;
36-
import org.elasticsearch.rest.action.RestBuilderListener;
32+
import org.elasticsearch.rest.action.RestToXContentListener;
3733

3834
import java.io.IOException;
3935
import java.util.Map;
4036

4137
import static org.elasticsearch.rest.RestRequest.Method.GET;
4238
import static org.elasticsearch.rest.RestRequest.Method.POST;
43-
import static org.elasticsearch.rest.RestStatus.OK;
4439

4540
public class RestClearIndicesCacheAction extends BaseRestHandler {
4641
public RestClearIndicesCacheAction(Settings settings, RestController controller) {
@@ -63,16 +58,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
6358
Strings.splitStringByCommaToArray(request.param("index")));
6459
clearIndicesCacheRequest.indicesOptions(IndicesOptions.fromRequest(request, clearIndicesCacheRequest.indicesOptions()));
6560
fromRequest(request, clearIndicesCacheRequest);
66-
return channel ->
67-
client.admin().indices().clearCache(clearIndicesCacheRequest, new RestBuilderListener<ClearIndicesCacheResponse>(channel) {
68-
@Override
69-
public RestResponse buildResponse(ClearIndicesCacheResponse response, XContentBuilder builder) throws Exception {
70-
builder.startObject();
71-
response.toXContent(builder, request);
72-
builder.endObject();
73-
return new BytesRestResponse(OK, builder);
74-
}
75-
});
61+
return channel -> client.admin().indices().clearCache(clearIndicesCacheRequest, new RestToXContentListener<>(channel));
7662
}
7763

7864
@Override

0 commit comments

Comments
 (0)