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
elastic · Mar 23, 2018 · 4a8099c · 4a8099c
1 parent 8328b9c
commit 4a8099c
Show file tree

Hide file tree

Showing 20 changed files with 105 additions and 243 deletions.
diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/recovery/RecoveryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/recovery/RecoveryResponse.java
@@ -24,7 +24,6 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.indices.recovery.RecoveryState;
 
@@ -37,9 +36,8 @@
 /**
  * Information regarding the recovery state of indices and their associated shards.
  */
-public class RecoveryResponse extends BroadcastResponse implements ToXContentFragment {
+public class RecoveryResponse extends BroadcastResponse {
 
-    private boolean detailed = false;
     private Map<String, List<RecoveryState>> shardRecoveryStates = new HashMap<>();
 
     public RecoveryResponse() { }
@@ -51,36 +49,26 @@ public RecoveryResponse() { }
      * @param totalShards       Total count of shards seen
      * @param successfulShards  Count of shards successfully processed
      * @param failedShards      Count of shards which failed to process
-     * @param detailed          Display detailed metrics
      * @param shardRecoveryStates    Map of indices to shard recovery information
      * @param shardFailures     List of failures processing shards
      */
-    public RecoveryResponse(int totalShards, int successfulShards, int failedShards, boolean detailed,
-                            Map<String, List<RecoveryState>> shardRecoveryStates,
+    public RecoveryResponse(int totalShards, int successfulShards, int failedShards, Map<String, List<RecoveryState>> shardRecoveryStates,
                             List<DefaultShardOperationFailedException> shardFailures) {
         super(totalShards, successfulShards, failedShards, shardFailures);
         this.shardRecoveryStates = shardRecoveryStates;
-        this.detailed = detailed;
     }
 
     public boolean hasRecoveries() {
         return shardRecoveryStates.size() > 0;
     }
 
-    public boolean detailed() {
-        return detailed;
-    }
-
-    public void detailed(boolean detailed) {
-        this.detailed = detailed;
-    }
-
     public Map<String, List<RecoveryState>> shardRecoveryStates() {
         return shardRecoveryStates;
     }
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
         if (hasRecoveries()) {
             for (String index : shardRecoveryStates.keySet()) {
                 List<RecoveryState> recoveryStates = shardRecoveryStates.get(index);
@@ -98,6 +86,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
                 builder.endObject();
             }
         }
+        builder.endObject();
         return builder;
     }
 
@@ -133,4 +122,4 @@ public void readFrom(StreamInput in) throws IOException {
     public String toString() {
         return Strings.toString(this, true, true);
     }
-}
+}
diff --git a/...rc/main/java/org/elasticsearch/action/admin/indices/recovery/TransportRecoveryAction.java b/...rc/main/java/org/elasticsearch/action/admin/indices/recovery/TransportRecoveryAction.java
@@ -87,7 +87,7 @@ protected RecoveryResponse newResponse(RecoveryRequest request, int totalShards,
                 shardResponses.get(indexName).add(recoveryState);
             }
         }
-        return new RecoveryResponse(totalShards, successfulShards, failedShards, request.detailed(), shardResponses, shardFailures);
+        return new RecoveryResponse(totalShards, successfulShards, failedShards, shardResponses, shardFailures);
     }
 
     @Override
@@ -118,4 +118,4 @@ protected ClusterBlockException checkGlobalBlock(ClusterState state, RecoveryReq
     protected ClusterBlockException checkRequestBlock(ClusterState state, RecoveryRequest request, String[] concreteIndices) {
         return state.blocks().indicesBlockedException(ClusterBlockLevel.READ, concreteIndices);
     }
-}
+}
diff --git a/...src/main/java/org/elasticsearch/action/admin/indices/segments/IndicesSegmentResponse.java b/...src/main/java/org/elasticsearch/action/admin/indices/segments/IndicesSegmentResponse.java
@@ -29,7 +29,6 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.unit.ByteSizeValue;
-import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.index.engine.Segment;
 
@@ -43,7 +42,7 @@
 import java.util.Map;
 import java.util.Set;
 
-public class IndicesSegmentResponse extends BroadcastResponse implements ToXContentFragment {
+public class IndicesSegmentResponse extends BroadcastResponse {
 
     private ShardSegments[] shards;
 
@@ -103,7 +102,7 @@ public void writeTo(StreamOutput out) throws IOException {
     }
 
     @Override
-    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+    protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(Fields.INDICES);
 
         for (IndexSegments indexSegments : getIndices().values()) {
@@ -173,10 +172,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         }
 
         builder.endObject();
-        return builder;
     }
 
-    static void toXContent(XContentBuilder builder, Sort sort) throws IOException {
+    private static void toXContent(XContentBuilder builder, Sort sort) throws IOException {
         builder.startArray("sort");
         for (SortField field : sort.getSort()) {
             builder.startObject();
@@ -195,7 +193,7 @@ static void toXContent(XContentBuilder builder, Sort sort) throws IOException {
         builder.endArray();
     }
 
-    static void toXContent(XContentBuilder builder, Accountable tree) throws IOException {
+    private static void toXContent(XContentBuilder builder, Accountable tree) throws IOException {
         builder.startObject();
         builder.field(Fields.DESCRIPTION, tree.toString());
         builder.humanReadableField(Fields.SIZE_IN_BYTES, Fields.SIZE, new ByteSizeValue(tree.ramBytesUsed()));

diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponse.java
@@ -25,9 +25,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.XContentFactory;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -39,7 +37,7 @@
 
 import static java.util.Collections.unmodifiableMap;
 
-public class IndicesStatsResponse extends BroadcastResponse implements ToXContentFragment {
+public class IndicesStatsResponse extends BroadcastResponse {
 
     private ShardStats[] shards;
 
@@ -147,15 +145,14 @@ public void writeTo(StreamOutput out) throws IOException {
     }
 
     @Override
-    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+    protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
         final String level = params.param("level", "indices");
         final boolean isLevelValid =
             "cluster".equalsIgnoreCase(level) || "indices".equalsIgnoreCase(level) || "shards".equalsIgnoreCase(level);
         if (!isLevelValid) {
             throw new IllegalArgumentException("level parameter must be one of [cluster] or [indices] or [shards] but was [" + level + "]");
         }
 
-
         builder.startObject("_all");
 
         builder.startObject("primaries");
@@ -198,8 +195,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             }
             builder.endObject();
         }
-
-        return builder;
     }
 
     static final class Fields {
@@ -209,14 +204,6 @@ static final class Fields {
 
     @Override
     public String toString() {
-        try {
-            XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint();
-            builder.startObject();
-            toXContent(builder, EMPTY_PARAMS);
-            builder.endObject();
-            return Strings.toString(builder);
-        } catch (IOException e) {
-            return "{ \"error\" : \"" + e.getMessage() + "\"}";
-        }
+        return Strings.toString(this, true, false);
     }
 }
diff --git a/...c/main/java/org/elasticsearch/action/admin/indices/upgrade/get/UpgradeStatusResponse.java b/...c/main/java/org/elasticsearch/action/admin/indices/upgrade/get/UpgradeStatusResponse.java
@@ -23,7 +23,6 @@
 import org.elasticsearch.action.support.broadcast.BroadcastResponse;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
@@ -34,7 +33,7 @@
 import java.util.Map;
 import java.util.Set;
 
-public class UpgradeStatusResponse extends BroadcastResponse implements ToXContentFragment {
+public class UpgradeStatusResponse extends BroadcastResponse {
     private ShardUpgradeStatus[] shards;
 
     private Map<String, IndexUpgradeStatus> indicesUpgradeStatus;
@@ -116,6 +115,7 @@ public long getToUpgradeBytesAncient() {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
         builder.byteSizeField(Fields.SIZE_IN_BYTES, Fields.SIZE, getTotalBytes());
         builder.byteSizeField(Fields.SIZE_TO_UPGRADE_IN_BYTES, Fields.SIZE_TO_UPGRADE, getToUpgradeBytes());
         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
             }
             builder.endObject();
         }
+        builder.endObject();
         return builder;
     }
 

diff --git a/...er/src/main/java/org/elasticsearch/action/admin/indices/upgrade/post/UpgradeResponse.java b/...er/src/main/java/org/elasticsearch/action/admin/indices/upgrade/post/UpgradeResponse.java
@@ -25,6 +25,7 @@
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.util.HashMap;
@@ -74,6 +75,18 @@ public void writeTo(StreamOutput out) throws IOException {
         }
     }
 
+    @Override
+    protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject("upgraded_indices");
+        for (Map.Entry<String, Tuple<Version, String>> entry : versions.entrySet()) {
+            builder.startObject(entry.getKey());
+            builder.field("upgrade_version", entry.getValue().v1());
+            builder.field("oldest_lucene_segment_version", entry.getValue().v2());
+            builder.endObject();
+        }
+        builder.endObject();
+    }
+
     /**
      * Returns the highest upgrade version of the node that performed metadata upgrade and the
      * the version of the oldest lucene segment for each index that was upgraded.

diff --git a/...ain/java/org/elasticsearch/action/admin/indices/validate/query/ValidateQueryResponse.java b/...ain/java/org/elasticsearch/action/admin/indices/validate/query/ValidateQueryResponse.java
@@ -23,6 +23,7 @@
 import org.elasticsearch.action.support.broadcast.BroadcastResponse;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -38,8 +39,15 @@
  */
 public class ValidateQueryResponse extends BroadcastResponse {
 
+    public static final String INDEX_FIELD = "index";
+    public static final String SHARD_FIELD = "shard";
+    public static final String VALID_FIELD = "valid";
+    public static final String EXPLANATIONS_FIELD = "explanations";
+    public static final String ERROR_FIELD = "error";
+    public static final String EXPLANATION_FIELD = "explanation";
+
     private boolean valid;
-    
+
     private List<QueryExplanation> queryExplanations;
 
     ValidateQueryResponse() {
@@ -96,4 +104,30 @@ public void writeTo(StreamOutput out) throws IOException {
         }
 
     }
+
+    @Override
+    protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
+        builder.field(VALID_FIELD, isValid());
+        if (getQueryExplanation() != null && !getQueryExplanation().isEmpty()) {
+            builder.startArray(EXPLANATIONS_FIELD);
+            for (QueryExplanation explanation : getQueryExplanation()) {
+                builder.startObject();
+                if (explanation.getIndex() != null) {
+                    builder.field(INDEX_FIELD, explanation.getIndex());
+                }
+                if(explanation.getShard() >= 0) {
+                    builder.field(SHARD_FIELD, explanation.getShard());
+                }
+                builder.field(VALID_FIELD, explanation.isValid());
+                if (explanation.getError() != null) {
+                    builder.field(ERROR_FIELD, explanation.getError());
+                }
+                if (explanation.getExplanation() != null) {
+                    builder.field(EXPLANATION_FIELD, explanation.getExplanation());
+                }
+                builder.endObject();
+            }
+            builder.endArray();
+        }
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/action/support/broadcast/BroadcastResponse.java b/server/src/main/java/org/elasticsearch/action/support/broadcast/BroadcastResponse.java
@@ -25,7 +25,7 @@
 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.ToXContentFragment;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.action.RestActions;
@@ -40,7 +40,7 @@
 /**
  * Base class for all broadcast operation based responses.
  */
-public class BroadcastResponse extends ActionResponse implements ToXContentFragment {
+public class BroadcastResponse extends ActionResponse implements ToXContentObject {
 
     public static final DefaultShardOperationFailedException[] EMPTY = new DefaultShardOperationFailedException[0];
 
@@ -149,7 +149,16 @@ public void writeTo(StreamOutput out) throws IOException {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
         RestActions.buildBroadcastShardsHeader(builder, params, this);
+        addCustomXContentFields(builder, params);
+        builder.endObject();
         return builder;
     }
+
+    /**
+     * Override in subclass to add custom fields following the common `_shards` field
+     */
+    protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
+    }
 }
diff --git a/...rc/main/java/org/elasticsearch/rest/action/admin/indices/RestClearIndicesCacheAction.java b/...rc/main/java/org/elasticsearch/rest/action/admin/indices/RestClearIndicesCacheAction.java
@@ -20,24 +20,19 @@
 package org.elasticsearch.rest.action.admin.indices;
 
 import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest;
-import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheResponse;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.BaseRestHandler;
-import org.elasticsearch.rest.BytesRestResponse;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
-import org.elasticsearch.rest.RestResponse;
-import org.elasticsearch.rest.action.RestBuilderListener;
+import org.elasticsearch.rest.action.RestToXContentListener;
 
 import java.io.IOException;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
 import static org.elasticsearch.rest.RestRequest.Method.POST;
-import static org.elasticsearch.rest.RestStatus.OK;
 
 public class RestClearIndicesCacheAction extends BaseRestHandler {
 
@@ -61,16 +56,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
                 Strings.splitStringByCommaToArray(request.param("index")));
         clearIndicesCacheRequest.indicesOptions(IndicesOptions.fromRequest(request, clearIndicesCacheRequest.indicesOptions()));
         fromRequest(request, clearIndicesCacheRequest);
-        return channel ->
-                client.admin().indices().clearCache(clearIndicesCacheRequest, new RestBuilderListener<ClearIndicesCacheResponse>(channel) {
-            @Override
-            public RestResponse buildResponse(ClearIndicesCacheResponse response, XContentBuilder builder) throws Exception {
-                builder.startObject();
-                response.toXContent(builder, request);
-                builder.endObject();
-                return new BytesRestResponse(OK, builder);
-            }
-        });
+        return channel -> client.admin().indices().clearCache(clearIndicesCacheRequest, new RestToXContentListener<>(channel));
     }
 
     @Override