diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java index 8cec474db6fb1..81616f95344ff 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java @@ -8,26 +8,32 @@ package org.elasticsearch.action.admin.cluster.reroute; -import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.support.master.IsAcknowledgeSupplier; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.routing.allocation.RoutingExplanations; +import org.elasticsearch.common.collect.Iterators; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.logging.DeprecationCategory; import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.xcontent.ChunkedToXContent; +import org.elasticsearch.common.xcontent.ChunkedToXContentHelper; +import org.elasticsearch.common.xcontent.ChunkedToXContentObject; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.rest.action.search.RestSearchAction; -import org.elasticsearch.xcontent.ToXContentObject; -import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.ToXContent; import java.io.IOException; +import java.util.Collections; +import java.util.Iterator; import java.util.Objects; +import static org.elasticsearch.action.support.master.AcknowledgedResponse.ACKNOWLEDGED_KEY; + /** * Response returned after a cluster reroute request */ -public class ClusterRerouteResponse extends AcknowledgedResponse implements ToXContentObject { +public class ClusterRerouteResponse extends ActionResponse implements IsAcknowledgeSupplier, ChunkedToXContentObject { private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestSearchAction.class); public static final String STATE_FIELD_DEPRECATION_MESSAGE = "The [state] field in the response to the reroute API is deprecated " @@ -38,15 +44,17 @@ public class ClusterRerouteResponse extends AcknowledgedResponse implements ToXC */ private final ClusterState state; private final RoutingExplanations explanations; + private final boolean acknowledged; ClusterRerouteResponse(StreamInput in) throws IOException { super(in); + acknowledged = in.readBoolean(); state = ClusterState.readFrom(in, null); explanations = RoutingExplanations.readFrom(in); } ClusterRerouteResponse(boolean acknowledged, ClusterState state, RoutingExplanations explanations) { - super(acknowledged); + this.acknowledged = acknowledged; this.state = state; this.explanations = explanations; } @@ -62,27 +70,45 @@ public RoutingExplanations getExplanations() { return this.explanations; } + @Override + public final boolean isAcknowledged() { + return acknowledged; + } + @Override public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); + out.writeBoolean(acknowledged); state.writeTo(out); RoutingExplanations.writeTo(explanations, out); } + private boolean emitState(ToXContent.Params params) { + return Objects.equals(params.param("metric"), "none") == false; + } + @Override - protected void addCustomFields(XContentBuilder builder, Params params) throws IOException { - if (Objects.equals(params.param("metric"), "none") == false) { - if (builder.getRestApiVersion() != RestApiVersion.V_7) { - deprecationLogger.critical(DeprecationCategory.API, "reroute_cluster_state", STATE_FIELD_DEPRECATION_MESSAGE); - } - builder.startObject("state"); - // TODO this should be chunked, see #89838 - ChunkedToXContent.wrapAsToXContent(state).toXContent(builder, params); - builder.endObject(); + public Iterator toXContentChunked(ToXContent.Params outerParams) { + if (emitState(outerParams)) { + deprecationLogger.critical(DeprecationCategory.API, "reroute_cluster_state", STATE_FIELD_DEPRECATION_MESSAGE); } + return toXContentChunkedV7(outerParams); + } - if (params.paramAsBoolean("explain", false)) { - explanations.toXContent(builder, params); - } + @Override + public Iterator toXContentChunkedV7(ToXContent.Params outerParams) { + return Iterators.concat( + Iterators.single((builder, params) -> builder.startObject().field(ACKNOWLEDGED_KEY, isAcknowledged())), + emitState(outerParams) + ? ChunkedToXContentHelper.wrapWithObject("state", state.toXContentChunked(outerParams)) + : Collections.emptyIterator(), + Iterators.single((builder, params) -> { + if (params.paramAsBoolean("explain", false)) { + explanations.toXContent(builder, params); + } + + builder.endObject(); + return builder; + }) + ); } } diff --git a/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedRequestBuilder.java index 013a922e2ef86..2f445e4d8c3ce 100644 --- a/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedRequestBuilder.java @@ -7,6 +7,7 @@ */ package org.elasticsearch.action.support.master; +import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.ActionType; import org.elasticsearch.client.internal.ElasticsearchClient; import org.elasticsearch.core.TimeValue; @@ -16,7 +17,7 @@ */ public abstract class AcknowledgedRequestBuilder< Request extends AcknowledgedRequest, - Response extends AcknowledgedResponse, + Response extends ActionResponse & IsAcknowledgeSupplier, RequestBuilder extends AcknowledgedRequestBuilder> extends MasterNodeOperationRequestBuilder< Request, Response, diff --git a/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java b/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java index b019782b27d74..d5ee3f0549d23 100644 --- a/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java +++ b/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java @@ -25,13 +25,14 @@ /** * A response that indicates that a request has been acknowledged */ -public class AcknowledgedResponse extends ActionResponse implements ToXContentObject { +public class AcknowledgedResponse extends ActionResponse implements IsAcknowledgeSupplier, ToXContentObject { public static final AcknowledgedResponse TRUE = new AcknowledgedResponse(true); public static final AcknowledgedResponse FALSE = new AcknowledgedResponse(false); - private static final ParseField ACKNOWLEDGED = new ParseField("acknowledged"); + public static final String ACKNOWLEDGED_KEY = "acknowledged"; + private static final ParseField ACKNOWLEDGED = new ParseField(ACKNOWLEDGED_KEY); protected static void declareAcknowledgedField(ConstructingObjectParser objectParser) { objectParser.declareField( @@ -65,6 +66,7 @@ protected AcknowledgedResponse(boolean acknowledged) { * Returns whether the response is acknowledged or not * @return true if the response is acknowledged, false otherwise */ + @Override public final boolean isAcknowledged() { return acknowledged; } @@ -77,7 +79,7 @@ public void writeTo(StreamOutput out) throws IOException { @Override public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - builder.field(ACKNOWLEDGED.getPreferredName(), isAcknowledged()); + builder.field(ACKNOWLEDGED_KEY, isAcknowledged()); addCustomFields(builder, params); builder.endObject(); return builder; diff --git a/server/src/main/java/org/elasticsearch/action/support/master/IsAcknowledgeSupplier.java b/server/src/main/java/org/elasticsearch/action/support/master/IsAcknowledgeSupplier.java new file mode 100644 index 0000000000000..ed3cb353afe93 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/support/master/IsAcknowledgeSupplier.java @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.action.support.master; + +public interface IsAcknowledgeSupplier { + boolean isAcknowledged(); +} diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/ChunkedToXContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/ChunkedToXContent.java index 1e40fbef65a23..11e7b48834086 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/ChunkedToXContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/ChunkedToXContent.java @@ -8,6 +8,7 @@ package org.elasticsearch.common.xcontent; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; @@ -29,6 +30,14 @@ public interface ChunkedToXContent { */ Iterator toXContentChunked(ToXContent.Params params); + /** + * Similar to {@link #toXContentChunked} but for the {@link RestApiVersion#V_7} API. Note that chunked response bodies cannot send + * deprecation warning headers once transmission has started, so implementations must check for deprecated feature use before returning. + */ + default Iterator toXContentChunkedV7(ToXContent.Params params) { + return toXContentChunked(params); + } + /** * Wraps the given instance in a {@link ToXContent} that will fully serialize the instance when serialized. * @param chunkedToXContent instance to wrap diff --git a/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java b/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java index b6583a22b8aa6..d6efaf43d2ec2 100644 --- a/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java +++ b/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.xcontent.ChunkedToXContent; import org.elasticsearch.core.IOUtils; import org.elasticsearch.core.Releasables; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.Streams; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; @@ -82,7 +83,9 @@ public void write(byte[] b, int off, int len) throws IOException { Streams.noCloseStream(out) ); - private final Iterator serialization = chunkedToXContent.toXContentChunked(params); + private final Iterator serialization = builder.getRestApiVersion() == RestApiVersion.V_7 + ? chunkedToXContent.toXContentChunkedV7(params) + : chunkedToXContent.toXContentChunked(params); private BytesStream target; diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterRerouteAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterRerouteAction.java index a04e89bba8d2d..1dca904c55829 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterRerouteAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterRerouteAction.java @@ -18,7 +18,7 @@ import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.RestToXContentListener; +import org.elasticsearch.rest.action.RestChunkedToXContentListener; import org.elasticsearch.xcontent.ObjectParser; import org.elasticsearch.xcontent.ObjectParser.ValueType; import org.elasticsearch.xcontent.ParseField; @@ -82,7 +82,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC if (metric == null) { request.params().put("metric", DEFAULT_METRICS); } - return channel -> client.admin().cluster().reroute(clusterRerouteRequest, new RestToXContentListener<>(channel)); + return channel -> client.admin().cluster().reroute(clusterRerouteRequest, new RestChunkedToXContentListener<>(channel)); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java index 98f8e7385a2f1..31f3f26263ead 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java @@ -26,14 +26,13 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.ToXContent; -import org.elasticsearch.xcontent.json.JsonXContent; import java.io.IOException; import java.util.List; import java.util.Map; import static org.elasticsearch.common.util.CollectionUtils.appendToCopy; -import static org.hamcrest.Matchers.equalTo; +import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; public class ClusterRerouteResponseTests extends ESTestCase { @@ -43,54 +42,45 @@ protected List filteredWarnings() { } public void testToXContent() throws IOException { - var clusterState = createClusterState(); - var clusterRerouteResponse = createClusterRerouteResponse(clusterState); - - var result = toXContent(clusterRerouteResponse, new ToXContent.MapParams(Map.of("metric", "none"))); - - assertThat(result, equalTo(XContentHelper.stripWhitespace(""" + assertXContent(createClusterRerouteResponse(createClusterState()), new ToXContent.MapParams(Map.of("metric", "none")), 2, """ { "acknowledged": true - }"""))); + }"""); } - public void testToXContentWithExplain() throws IOException { + public void testToXContentWithExplain() { var clusterState = createClusterState(); - var clusterRerouteResponse = createClusterRerouteResponse(clusterState); - - var result = toXContent(clusterRerouteResponse, new ToXContent.MapParams(Map.of("explain", "true", "metric", "none"))); - - assertThat(result, equalTo(XContentHelper.stripWhitespace(formatted(""" - { - "acknowledged": true, - "explanations": [ + assertXContent( + createClusterRerouteResponse(clusterState), + new ToXContent.MapParams(Map.of("explain", "true", "metric", "none")), + 2, + formatted(""" { - "command": "allocate_replica", - "parameters": { - "index": "index", - "shard": 0, - "node": "node0" - }, - "decisions": [ + "acknowledged": true, + "explanations": [ { - "decider": null, - "decision": "YES", - "explanation": "none" + "command": "allocate_replica", + "parameters": { + "index": "index", + "shard": 0, + "node": "node0" + }, + "decisions": [ + { + "decider": null, + "decision": "YES", + "explanation": "none" + } + ] } ] - } - ] - }""", clusterState.stateUUID())))); - + }""", clusterState.stateUUID()) + ); } - public void testToXContentWithDeprecatedClusterState() throws IOException { + public void testToXContentWithDeprecatedClusterState() { var clusterState = createClusterState(); - var clusterRerouteResponse = createClusterRerouteResponse(clusterState); - - var result = toXContent(clusterRerouteResponse, ToXContent.EMPTY_PARAMS); - - assertThat(result, equalTo(XContentHelper.stripWhitespace(formatted(""" + assertXContent(createClusterRerouteResponse(clusterState), ToXContent.EMPTY_PARAMS, 32, formatted(""" { "acknowledged": true, "state": { @@ -185,77 +175,87 @@ public void testToXContentWithDeprecatedClusterState() throws IOException { } } } - }""", clusterState.stateUUID(), clusterState.getNodes().get("node0").getEphemeralId(), Version.CURRENT.id)))); + }""", clusterState.stateUUID(), clusterState.getNodes().get("node0").getEphemeralId(), Version.CURRENT.id)); } - public void testToXContentWithDeprecatedClusterStateAndMetadata() throws IOException { - var clusterState = createClusterState(); - var clusterRerouteResponse = createClusterRerouteResponse(clusterState); - - var result = toXContent( - clusterRerouteResponse, - new ToXContent.MapParams(Map.of("metric", "metadata", "settings_filter", "index.number*,index.version.created")) - ); - - assertThat(result, equalTo(XContentHelper.stripWhitespace(""" - { - "acknowledged" : true, - "state" : { - "cluster_uuid" : "_na_", - "metadata" : { - "cluster_uuid" : "_na_", - "cluster_uuid_committed" : false, - "cluster_coordination" : { - "term" : 0, - "last_committed_config" : [ ], - "last_accepted_config" : [ ], - "voting_config_exclusions" : [ ] - }, - "templates" : { }, - "indices" : { - "index" : { - "version" : 1, - "mapping_version" : 1, - "settings_version" : 1, - "aliases_version" : 1, - "routing_num_shards" : 1, - "state" : "open", - "settings" : { + public void testToXContentWithDeprecatedClusterStateAndMetadata() { + assertXContent( + createClusterRerouteResponse(createClusterState()), + new ToXContent.MapParams(Map.of("metric", "metadata", "settings_filter", "index.number*,index.version.created")), + 19, + """ + { + "acknowledged" : true, + "state" : { + "cluster_uuid" : "_na_", + "metadata" : { + "cluster_uuid" : "_na_", + "cluster_uuid_committed" : false, + "cluster_coordination" : { + "term" : 0, + "last_committed_config" : [ ], + "last_accepted_config" : [ ], + "voting_config_exclusions" : [ ] + }, + "templates" : { }, + "indices" : { "index" : { - "max_script_fields" : "10", - "shard" : { - "check_on_startup" : "true" + "version" : 1, + "mapping_version" : 1, + "settings_version" : 1, + "aliases_version" : 1, + "routing_num_shards" : 1, + "state" : "open", + "settings" : { + "index" : { + "max_script_fields" : "10", + "shard" : { + "check_on_startup" : "true" + } + } + }, + "mappings" : { }, + "aliases" : [ ], + "primary_terms" : { + "0" : 0 + }, + "in_sync_allocations" : { + "0" : [ ] + }, + "rollover_info" : { }, + "system" : false, + "timestamp_range" : { + "shards" : [ ] } } }, - "mappings" : { }, - "aliases" : [ ], - "primary_terms" : { - "0" : 0 - }, - "in_sync_allocations" : { - "0" : [ ] + "index-graveyard" : { + "tombstones" : [ ] }, - "rollover_info" : { }, - "system" : false, - "timestamp_range" : { - "shards" : [ ] - } + "reserved_state":{} } - }, - "index-graveyard" : { - "tombstones" : [ ] - }, - "reserved_state":{} - } - } - }"""))); + } + }""" + ); } - private static String toXContent(ClusterRerouteResponse clusterRerouteResponse, ToXContent.Params params) throws IOException { - var builder = JsonXContent.contentBuilder().prettyPrint(); - clusterRerouteResponse.toXContent(builder, params); - return XContentHelper.stripWhitespace(Strings.toString(builder)); + private static void assertXContent(ClusterRerouteResponse response, ToXContent.Params params, int expectedChunks, String expectedBody) { + try { + var builder = jsonBuilder(); + if (randomBoolean()) { + builder.prettyPrint(); + } + int actualChunks = 0; + final var iterator = response.toXContentChunked(params); + while (iterator.hasNext()) { + iterator.next().toXContent(builder, params); + actualChunks += 1; + } + assertEquals(XContentHelper.stripWhitespace(expectedBody), XContentHelper.stripWhitespace(Strings.toString(builder))); + assertEquals(expectedChunks, actualChunks); + } catch (IOException e) { + throw new AssertionError("unexpected", e); + } } private static ClusterRerouteResponse createClusterRerouteResponse(ClusterState clusterState) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java index d0b0d00fa47f7..d8da8dd83c4e8 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java +++ b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java @@ -27,7 +27,7 @@ import org.elasticsearch.action.support.DefaultShardOperationFailedException; import org.elasticsearch.action.support.broadcast.BaseBroadcastResponse; import org.elasticsearch.action.support.master.AcknowledgedRequestBuilder; -import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.action.support.master.IsAcknowledgeSupplier; import org.elasticsearch.cluster.block.ClusterBlock; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.metadata.IndexMetadata; @@ -99,7 +99,7 @@ public static void assertNoTimeout(ClusterHealthResponse response) { assertThat("ClusterHealthResponse has timed out - returned: [" + response + "]", response.isTimedOut(), is(false)); } - public static void assertAcked(AcknowledgedResponse response) { + public static void assertAcked(IsAcknowledgeSupplier response) { assertThat(response.getClass().getSimpleName() + " failed - not acked", response.isAcknowledged(), equalTo(true)); }