Skip to content

Commit 91d69d9

Browse files
author
Tyler Smalley
authored
Revert "Chunked encoding for RestGetIndicesAction (#92016)" (#92033)
This reverts commit 75de8f8 due to #92032 and elastic/kibana#146758
1 parent 2b8fde6 commit 91d69d9

File tree

6 files changed

+66
-77
lines changed

6 files changed

+66
-77
lines changed

‎modules/transport-netty4/src/javaRestTest/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import static org.elasticsearch.rest.RestStatus.OK;
2626
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
2727
import static org.hamcrest.Matchers.greaterThan;
28-
import static org.hamcrest.Matchers.nullValue;
2928

3029
public class Netty4HeadBodyIsEmptyIT extends ESRestTestCase {
3130
public void testHeadRoot() throws IOException {
@@ -60,8 +59,8 @@ public void testDocumentExists() throws IOException {
6059

6160
public void testIndexExists() throws IOException {
6261
createTestDoc();
63-
headTestCase("/test", emptyMap(), nullValue(Integer.class));
64-
headTestCase("/test", singletonMap("pretty", "true"), nullValue(Integer.class));
62+
headTestCase("/test", emptyMap(), greaterThan(0));
63+
headTestCase("/test", singletonMap("pretty", "true"), greaterThan(0));
6564
}
6665

6766
public void testAliasExists() throws IOException {
@@ -178,8 +177,7 @@ private void headTestCase(
178177
request.setOptions(expectWarnings(expectedWarnings));
179178
Response response = client().performRequest(request);
180179
assertEquals(expectedStatusCode, response.getStatusLine().getStatusCode());
181-
final var contentLength = response.getHeader("Content-Length");
182-
assertThat(contentLength == null ? null : Integer.valueOf(contentLength), matcher);
180+
assertThat(Integer.valueOf(response.getHeader("Content-Length")), matcher);
183181
assertNull("HEAD requests shouldn't have a response body but " + url + " did", response.getEntity());
184182
}
185183

‎server/src/main/java/org/elasticsearch/action/ActionModule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
768768
registerHandler.accept(new RestResetFeatureStateAction());
769769
registerHandler.accept(new RestGetFeatureUpgradeStatusAction());
770770
registerHandler.accept(new RestPostFeatureUpgradeAction());
771-
registerHandler.accept(new RestGetIndicesAction());
771+
registerHandler.accept(new RestGetIndicesAction(threadPool));
772772
registerHandler.accept(new RestIndicesStatsAction());
773773
registerHandler.accept(new RestIndicesSegmentsAction());
774774
registerHandler.accept(new RestIndicesShardStoresAction());

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

Lines changed: 47 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,16 @@
1313
import org.elasticsearch.cluster.metadata.AliasMetadata;
1414
import org.elasticsearch.cluster.metadata.MappingMetadata;
1515
import org.elasticsearch.common.Strings;
16-
import org.elasticsearch.common.collect.Iterators;
1716
import org.elasticsearch.common.io.stream.StreamInput;
1817
import org.elasticsearch.common.io.stream.StreamOutput;
1918
import org.elasticsearch.common.settings.Settings;
20-
import org.elasticsearch.common.xcontent.ChunkedToXContent;
2119
import org.elasticsearch.core.RestApiVersion;
2220
import org.elasticsearch.index.mapper.MapperService;
23-
import org.elasticsearch.xcontent.ToXContent;
21+
import org.elasticsearch.xcontent.ToXContentObject;
22+
import org.elasticsearch.xcontent.XContentBuilder;
2423

2524
import java.io.IOException;
2625
import java.util.Arrays;
27-
import java.util.Iterator;
2826
import java.util.List;
2927
import java.util.Map;
3028
import java.util.Objects;
@@ -35,7 +33,7 @@
3533
/**
3634
* A response for a get index action.
3735
*/
38-
public class GetIndexResponse extends ActionResponse implements ChunkedToXContent {
36+
public class GetIndexResponse extends ActionResponse implements ToXContentObject {
3937

4038
private Map<String, MappingMetadata> mappings = Map.of();
4139
private Map<String, List<AliasMetadata>> aliases = Map.of();
@@ -180,58 +178,59 @@ public void writeTo(StreamOutput out) throws IOException {
180178
}
181179

182180
@Override
183-
public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params ignored) {
184-
return Iterators.concat(
185-
Iterators.single((builder, params) -> builder.startObject()),
186-
Arrays.stream(indices).<ToXContent>map(index -> (builder, params) -> {
181+
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
182+
builder.startObject();
183+
{
184+
for (final String index : indices) {
187185
builder.startObject(index);
188-
189-
builder.startObject("aliases");
190-
List<AliasMetadata> indexAliases = aliases.get(index);
191-
if (indexAliases != null) {
192-
for (final AliasMetadata alias : indexAliases) {
193-
AliasMetadata.Builder.toXContent(alias, builder, params);
186+
{
187+
builder.startObject("aliases");
188+
List<AliasMetadata> indexAliases = aliases.get(index);
189+
if (indexAliases != null) {
190+
for (final AliasMetadata alias : indexAliases) {
191+
AliasMetadata.Builder.toXContent(alias, builder, params);
192+
}
194193
}
195-
}
196-
builder.endObject();
194+
builder.endObject();
197195

198-
MappingMetadata indexMappings = mappings.get(index);
199-
if (indexMappings == null) {
200-
builder.startObject("mappings").endObject();
201-
} else {
202-
if (builder.getRestApiVersion() == RestApiVersion.V_7
203-
&& params.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY)) {
204-
builder.startObject("mappings");
205-
builder.field(MapperService.SINGLE_MAPPING_NAME, indexMappings.sourceAsMap());
206-
builder.endObject();
196+
MappingMetadata indexMappings = mappings.get(index);
197+
if (indexMappings == null) {
198+
builder.startObject("mappings").endObject();
207199
} else {
208-
builder.field("mappings", indexMappings.sourceAsMap());
200+
if (builder.getRestApiVersion() == RestApiVersion.V_7
201+
&& params.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY)) {
202+
builder.startObject("mappings");
203+
builder.field(MapperService.SINGLE_MAPPING_NAME, indexMappings.sourceAsMap());
204+
builder.endObject();
205+
} else {
206+
builder.field("mappings", indexMappings.sourceAsMap());
207+
}
209208
}
210-
}
211209

212-
builder.startObject("settings");
213-
Settings indexSettings = settings.get(index);
214-
if (indexSettings != null) {
215-
indexSettings.toXContent(builder, params);
216-
}
217-
builder.endObject();
218-
219-
Settings defaultIndexSettings = defaultSettings.get(index);
220-
if (defaultIndexSettings != null && defaultIndexSettings.isEmpty() == false) {
221-
builder.startObject("defaults");
222-
defaultIndexSettings.toXContent(builder, params);
210+
builder.startObject("settings");
211+
Settings indexSettings = settings.get(index);
212+
if (indexSettings != null) {
213+
indexSettings.toXContent(builder, params);
214+
}
223215
builder.endObject();
224-
}
225216

226-
String dataStream = dataStreams.get(index);
227-
if (dataStream != null) {
228-
builder.field("data_stream", dataStream);
229-
}
217+
Settings defaultIndexSettings = defaultSettings.get(index);
218+
if (defaultIndexSettings != null && defaultIndexSettings.isEmpty() == false) {
219+
builder.startObject("defaults");
220+
defaultIndexSettings.toXContent(builder, params);
221+
builder.endObject();
222+
}
230223

231-
return builder.endObject();
232-
}).iterator(),
233-
Iterators.single((builder, params) -> builder.endObject())
234-
);
224+
String dataStream = dataStreams.get(index);
225+
if (dataStream != null) {
226+
builder.field("data_stream", dataStream);
227+
}
228+
}
229+
builder.endObject();
230+
}
231+
}
232+
builder.endObject();
233+
return builder;
235234
}
236235

237236
@Override

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@
1717
import org.elasticsearch.core.RestApiVersion;
1818
import org.elasticsearch.rest.BaseRestHandler;
1919
import org.elasticsearch.rest.RestRequest;
20+
import org.elasticsearch.rest.action.DispatchingRestToXContentListener;
2021
import org.elasticsearch.rest.action.RestCancellableNodeClient;
21-
import org.elasticsearch.rest.action.RestChunkedToXContentListener;
22+
import org.elasticsearch.threadpool.ThreadPool;
2223

2324
import java.io.IOException;
2425
import java.util.List;
@@ -38,6 +39,12 @@ public class RestGetIndicesAction extends BaseRestHandler {
3839

3940
private static final Set<String> COMPATIBLE_RESPONSE_PARAMS = addToCopy(Settings.FORMAT_PARAMS, INCLUDE_TYPE_NAME_PARAMETER);
4041

42+
private final ThreadPool threadPool;
43+
44+
public RestGetIndicesAction(ThreadPool threadPool) {
45+
this.threadPool = threadPool;
46+
}
47+
4148
@Override
4249
public List<Route> routes() {
4350
return List.of(new Route(GET, "/{index}"), new Route(HEAD, "/{index}"));
@@ -69,7 +76,10 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
6976
final var httpChannel = request.getHttpChannel();
7077
return channel -> new RestCancellableNodeClient(client, httpChannel).admin()
7178
.indices()
72-
.getIndex(getIndexRequest, new RestChunkedToXContentListener<>(channel));
79+
.getIndex(
80+
getIndexRequest,
81+
new DispatchingRestToXContentListener<>(threadPool.executor(ThreadPool.Names.MANAGEMENT), channel, request)
82+
);
7383
}
7484

7585
/**

‎server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexResponseTests.java

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
import org.elasticsearch.common.settings.Settings;
1919
import org.elasticsearch.index.RandomCreateIndexGenerator;
2020
import org.elasticsearch.test.AbstractWireSerializingTestCase;
21-
import org.elasticsearch.xcontent.ToXContent;
2221

23-
import java.io.IOException;
2422
import java.util.ArrayList;
2523
import java.util.Collections;
2624
import java.util.Comparator;
@@ -29,9 +27,6 @@
2927
import java.util.Locale;
3028
import java.util.Map;
3129

32-
import static org.elasticsearch.xcontent.ToXContent.EMPTY_PARAMS;
33-
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
34-
3530
public class GetIndexResponseTests extends AbstractWireSerializingTestCase<GetIndexResponse> {
3631

3732
@Override
@@ -78,18 +73,4 @@ protected GetIndexResponse createTestInstance() {
7873
}
7974
return new GetIndexResponse(indices, mappings, aliases, settings, defaultSettings, dataStreams);
8075
}
81-
82-
public void testChunking() throws IOException {
83-
final var response = createTestInstance();
84-
85-
try (var builder = jsonBuilder()) {
86-
int chunkCount = 0;
87-
final var iterator = response.toXContentChunked(EMPTY_PARAMS);
88-
while (iterator.hasNext()) {
89-
iterator.next().toXContent(builder, ToXContent.EMPTY_PARAMS);
90-
chunkCount += 1;
91-
}
92-
assertEquals(response.getIndices().length + 2, chunkCount);
93-
} // closing the builder verifies that the XContent is well-formed
94-
}
9576
}

‎server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesActionTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package org.elasticsearch.rest.action.admin.indices;
1010

1111
import org.elasticsearch.client.internal.node.NodeClient;
12+
import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue;
1213
import org.elasticsearch.core.RestApiVersion;
1314
import org.elasticsearch.rest.RestRequest;
1415
import org.elasticsearch.test.ESTestCase;
@@ -36,7 +37,7 @@ public void testIncludeTypeNamesWarning() throws IOException {
3637
Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader)
3738
).withMethod(RestRequest.Method.GET).withPath("/some_index").withParams(params).build();
3839

39-
RestGetIndicesAction handler = new RestGetIndicesAction();
40+
RestGetIndicesAction handler = new RestGetIndicesAction(new DeterministicTaskQueue().getThreadPool());
4041
handler.prepareRequest(request, mock(NodeClient.class));
4142
assertCriticalWarnings(RestGetIndicesAction.TYPES_DEPRECATION_MESSAGE);
4243

@@ -57,7 +58,7 @@ public void testIncludeTypeNamesWarningExists() throws IOException {
5758
Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader)
5859
).withMethod(RestRequest.Method.HEAD).withPath("/some_index").withParams(params).build();
5960

60-
RestGetIndicesAction handler = new RestGetIndicesAction();
61+
RestGetIndicesAction handler = new RestGetIndicesAction(new DeterministicTaskQueue().getThreadPool());
6162
handler.prepareRequest(request, mock(NodeClient.class));
6263
}
6364
}

0 commit comments

Comments
 (0)