Skip to content

Commit 5fdd409

Browse files
original-brownbearoriginal-brownbear
committed
Remove unused NamedWriteableRegistry from a search REST actions
We don't need `NamedWriteableRegistry`to parse search requests any longer, this was an unused parameter. Removing it from search request parsing allows for removing it as a dependency from a number of places.
1 parent 08ad143 commit 5fdd409

File tree

26 files changed

+50
-186
lines changed

26 files changed

+50
-186
lines changed

‎modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustachePlugin.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public List<RestHandler> getRestHandlers(
6767
Predicate<NodeFeature> clusterSupportsFeature
6868
) {
6969
return Arrays.asList(
70-
new RestSearchTemplateAction(namedWriteableRegistry, clusterSupportsFeature),
70+
new RestSearchTemplateAction(clusterSupportsFeature),
7171
new RestMultiSearchTemplateAction(settings),
7272
new RestRenderSearchTemplateAction()
7373
);

‎modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import org.elasticsearch.action.search.SearchRequest;
1212
import org.elasticsearch.client.internal.node.NodeClient;
13-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1413
import org.elasticsearch.core.RestApiVersion;
1514
import org.elasticsearch.features.NodeFeature;
1615
import org.elasticsearch.rest.BaseRestHandler;
@@ -36,11 +35,9 @@ public class RestSearchTemplateAction extends BaseRestHandler {
3635

3736
private static final Set<String> RESPONSE_PARAMS = Set.of(TYPED_KEYS_PARAM, RestSearchAction.TOTAL_HITS_AS_INT_PARAM);
3837

39-
private final NamedWriteableRegistry namedWriteableRegistry;
4038
private final Predicate<NodeFeature> clusterSupportsFeature;
4139

42-
public RestSearchTemplateAction(NamedWriteableRegistry namedWriteableRegistry, Predicate<NodeFeature> clusterSupportsFeature) {
43-
this.namedWriteableRegistry = namedWriteableRegistry;
40+
public RestSearchTemplateAction(Predicate<NodeFeature> clusterSupportsFeature) {
4441
this.clusterSupportsFeature = clusterSupportsFeature;
4542
}
4643

@@ -73,7 +70,6 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
7370
searchRequest,
7471
request,
7572
null,
76-
namedWriteableRegistry,
7773
clusterSupportsFeature,
7874
size -> searchRequest.source().size(size)
7975
);

‎modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/RestSearchTemplateActionTests.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
package org.elasticsearch.script.mustache;
1010

11-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1211
import org.elasticsearch.core.RestApiVersion;
1312
import org.elasticsearch.rest.RestRequest;
1413
import org.elasticsearch.rest.action.search.RestSearchAction;
@@ -28,7 +27,7 @@ public final class RestSearchTemplateActionTests extends RestActionTestCase {
2827

2928
@Before
3029
public void setUpAction() {
31-
controller().registerHandler(new RestSearchTemplateAction(mock(NamedWriteableRegistry.class), nf -> false));
30+
controller().registerHandler(new RestSearchTemplateAction(nf -> false));
3231
verifyingClient.setExecuteVerifier((actionType, request) -> mock(SearchTemplateResponse.class));
3332
verifyingClient.setExecuteLocallyVerifier((actionType, request) -> mock(SearchTemplateResponse.class));
3433
}

‎modules/reindex/src/main/java/org/elasticsearch/reindex/AbstractBaseReindexRestHandler.java

+4-10
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.elasticsearch.action.support.ActiveShardCount;
1414
import org.elasticsearch.action.support.SubscribableListener;
1515
import org.elasticsearch.client.internal.node.NodeClient;
16-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1716
import org.elasticsearch.index.reindex.AbstractBulkByScrollRequest;
1817
import org.elasticsearch.index.reindex.BulkByScrollResponse;
1918
import org.elasticsearch.index.reindex.BulkByScrollTask;
@@ -39,15 +38,10 @@ protected AbstractBaseReindexRestHandler(A action) {
3938
this.action = action;
4039
}
4140

42-
protected RestChannelConsumer doPrepareRequest(
43-
RestRequest request,
44-
NamedWriteableRegistry namedWriteableRegistry,
45-
NodeClient client,
46-
boolean includeCreated,
47-
boolean includeUpdated
48-
) throws IOException {
41+
protected RestChannelConsumer doPrepareRequest(RestRequest request, NodeClient client, boolean includeCreated, boolean includeUpdated)
42+
throws IOException {
4943
// Build the internal request
50-
Request internal = setCommonOptions(request, buildRequest(request, namedWriteableRegistry));
44+
Request internal = setCommonOptions(request, buildRequest(request));
5145

5246
// Executes the request and waits for completion
5347
if (request.paramAsBoolean("wait_for_completion", true)) {
@@ -78,7 +72,7 @@ protected RestChannelConsumer doPrepareRequest(
7872
/**
7973
* Build the Request based on the RestRequest.
8074
*/
81-
protected abstract Request buildRequest(RestRequest request, NamedWriteableRegistry namedWriteableRegistry) throws IOException;
75+
protected abstract Request buildRequest(RestRequest request) throws IOException;
8276

8377
/**
8478
* Sets common options of {@link AbstractBulkByScrollRequest} requests.

‎modules/reindex/src/main/java/org/elasticsearch/reindex/AbstractBulkByQueryRestHandler.java

+1-10
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import org.elasticsearch.action.ActionType;
1212
import org.elasticsearch.action.search.SearchRequest;
1313
import org.elasticsearch.common.bytes.BytesReference;
14-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1514
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
1615
import org.elasticsearch.common.xcontent.XContentHelper;
1716
import org.elasticsearch.core.RestApiVersion;
@@ -45,7 +44,6 @@ protected AbstractBulkByQueryRestHandler(A action) {
4544
protected void parseInternalRequest(
4645
Request internal,
4746
RestRequest restRequest,
48-
NamedWriteableRegistry namedWriteableRegistry,
4947
Predicate<NodeFeature> clusterSupportsFeature,
5048
Map<String, Consumer<Object>> bodyConsumers
5149
) throws IOException {
@@ -58,14 +56,7 @@ protected void parseInternalRequest(
5856
IntConsumer sizeConsumer = restRequest.getRestApiVersion() == RestApiVersion.V_7
5957
? size -> setMaxDocsFromSearchSize(internal, size)
6058
: size -> failOnSizeSpecified();
61-
RestSearchAction.parseSearchRequest(
62-
searchRequest,
63-
restRequest,
64-
parser,
65-
namedWriteableRegistry,
66-
clusterSupportsFeature,
67-
sizeConsumer
68-
);
59+
RestSearchAction.parseSearchRequest(searchRequest, restRequest, parser, clusterSupportsFeature, sizeConsumer);
6960
}
7061

7162
searchRequest.source().size(restRequest.paramAsInt("scroll_size", searchRequest.source().size()));

‎modules/reindex/src/main/java/org/elasticsearch/reindex/ReindexPlugin.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ public List<RestHandler> getRestHandlers(
7676
Predicate<NodeFeature> clusterSupportsFeature
7777
) {
7878
return Arrays.asList(
79-
new RestReindexAction(namedWriteableRegistry, clusterSupportsFeature),
80-
new RestUpdateByQueryAction(namedWriteableRegistry, clusterSupportsFeature),
81-
new RestDeleteByQueryAction(namedWriteableRegistry, clusterSupportsFeature),
79+
new RestReindexAction(clusterSupportsFeature),
80+
new RestUpdateByQueryAction(clusterSupportsFeature),
81+
new RestDeleteByQueryAction(clusterSupportsFeature),
8282
new RestRethrottleAction(nodesInCluster)
8383
);
8484
}

‎modules/reindex/src/main/java/org/elasticsearch/reindex/RestDeleteByQueryAction.java

+4-7
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
package org.elasticsearch.reindex;
1010

1111
import org.elasticsearch.client.internal.node.NodeClient;
12-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1312
import org.elasticsearch.core.RestApiVersion;
1413
import org.elasticsearch.features.NodeFeature;
1514
import org.elasticsearch.index.reindex.DeleteByQueryAction;
@@ -31,12 +30,10 @@
3130
@ServerlessScope(Scope.PUBLIC)
3231
public class RestDeleteByQueryAction extends AbstractBulkByQueryRestHandler<DeleteByQueryRequest, DeleteByQueryAction> {
3332

34-
private final NamedWriteableRegistry namedWriteableRegistry;
3533
private final Predicate<NodeFeature> clusterSupportsFeature;
3634

37-
public RestDeleteByQueryAction(NamedWriteableRegistry namedWriteableRegistry, Predicate<NodeFeature> clusterSupportsFeature) {
35+
public RestDeleteByQueryAction(Predicate<NodeFeature> clusterSupportsFeature) {
3836
super(DeleteByQueryAction.INSTANCE);
39-
this.namedWriteableRegistry = namedWriteableRegistry;
4037
this.clusterSupportsFeature = clusterSupportsFeature;
4138
}
4239

@@ -58,11 +55,11 @@ public String getName() {
5855

5956
@Override
6057
public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
61-
return doPrepareRequest(request, namedWriteableRegistry, client, false, false);
58+
return doPrepareRequest(request, client, false, false);
6259
}
6360

6461
@Override
65-
protected DeleteByQueryRequest buildRequest(RestRequest request, NamedWriteableRegistry namedWriteableRegistry) throws IOException {
62+
protected DeleteByQueryRequest buildRequest(RestRequest request) throws IOException {
6663
/*
6764
* Passing the search request through DeleteByQueryRequest first allows
6865
* it to set its own defaults which differ from SearchRequest's
@@ -74,7 +71,7 @@ protected DeleteByQueryRequest buildRequest(RestRequest request, NamedWriteableR
7471
consumers.put("conflicts", o -> internal.setConflicts((String) o));
7572
consumers.put("max_docs", s -> setMaxDocsValidateIdentical(internal, ((Number) s).intValue()));
7673

77-
parseInternalRequest(internal, request, namedWriteableRegistry, clusterSupportsFeature, consumers);
74+
parseInternalRequest(internal, request, clusterSupportsFeature, consumers);
7875

7976
return internal;
8077
}

‎modules/reindex/src/main/java/org/elasticsearch/reindex/RestReindexAction.java

+3-6
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import org.elasticsearch.action.DocWriteRequest;
1212
import org.elasticsearch.client.internal.node.NodeClient;
13-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1413
import org.elasticsearch.features.NodeFeature;
1514
import org.elasticsearch.index.reindex.ReindexAction;
1615
import org.elasticsearch.index.reindex.ReindexRequest;
@@ -34,12 +33,10 @@
3433
@ServerlessScope(Scope.PUBLIC)
3534
public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexRequest, ReindexAction> implements RestRequestFilter {
3635

37-
private final NamedWriteableRegistry namedWriteableRegistry;
3836
private final Predicate<NodeFeature> clusterSupportsFeature;
3937

40-
public RestReindexAction(NamedWriteableRegistry namedWriteableRegistry, Predicate<NodeFeature> clusterSupportsFeature) {
38+
public RestReindexAction(Predicate<NodeFeature> clusterSupportsFeature) {
4139
super(ReindexAction.INSTANCE);
42-
this.namedWriteableRegistry = namedWriteableRegistry;
4340
this.clusterSupportsFeature = clusterSupportsFeature;
4441
}
4542

@@ -55,11 +52,11 @@ public String getName() {
5552

5653
@Override
5754
public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
58-
return doPrepareRequest(request, namedWriteableRegistry, client, true, true);
55+
return doPrepareRequest(request, client, true, true);
5956
}
6057

6158
@Override
62-
protected ReindexRequest buildRequest(RestRequest request, NamedWriteableRegistry namedWriteableRegistry) throws IOException {
59+
protected ReindexRequest buildRequest(RestRequest request) throws IOException {
6360
if (request.hasParam("pipeline")) {
6461
throw new IllegalArgumentException(
6562
"_reindex doesn't support [pipeline] as a query parameter. Specify it in the [dest] object instead."

‎modules/reindex/src/main/java/org/elasticsearch/reindex/RestUpdateByQueryAction.java

+4-7
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
package org.elasticsearch.reindex;
1010

1111
import org.elasticsearch.client.internal.node.NodeClient;
12-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1312
import org.elasticsearch.core.RestApiVersion;
1413
import org.elasticsearch.features.NodeFeature;
1514
import org.elasticsearch.index.reindex.UpdateByQueryAction;
@@ -32,12 +31,10 @@
3231
@ServerlessScope(Scope.PUBLIC)
3332
public class RestUpdateByQueryAction extends AbstractBulkByQueryRestHandler<UpdateByQueryRequest, UpdateByQueryAction> {
3433

35-
private final NamedWriteableRegistry namedWriteableRegistry;
3634
private final Predicate<NodeFeature> clusterSupportsFeature;
3735

38-
public RestUpdateByQueryAction(NamedWriteableRegistry namedWriteableRegistry, Predicate<NodeFeature> clusterSupportsFeature) {
36+
public RestUpdateByQueryAction(Predicate<NodeFeature> clusterSupportsFeature) {
3937
super(UpdateByQueryAction.INSTANCE);
40-
this.namedWriteableRegistry = namedWriteableRegistry;
4138
this.clusterSupportsFeature = clusterSupportsFeature;
4239
}
4340

@@ -58,11 +55,11 @@ public String getName() {
5855

5956
@Override
6057
public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
61-
return doPrepareRequest(request, namedWriteableRegistry, client, false, true);
58+
return doPrepareRequest(request, client, false, true);
6259
}
6360

6461
@Override
65-
protected UpdateByQueryRequest buildRequest(RestRequest request, NamedWriteableRegistry namedWriteableRegistry) throws IOException {
62+
protected UpdateByQueryRequest buildRequest(RestRequest request) throws IOException {
6663
if (request.getRestApiVersion() == RestApiVersion.V_7 && request.hasParam("type")) {
6764
request.param("type");
6865
}
@@ -78,7 +75,7 @@ protected UpdateByQueryRequest buildRequest(RestRequest request, NamedWriteableR
7875
consumers.put("script", o -> internal.setScript(Script.parse(o)));
7976
consumers.put("max_docs", s -> setMaxDocsValidateIdentical(internal, ((Number) s).intValue()));
8077

81-
parseInternalRequest(internal, request, namedWriteableRegistry, clusterSupportsFeature, consumers);
78+
parseInternalRequest(internal, request, clusterSupportsFeature, consumers);
8279

8380
internal.setPipeline(request.param("pipeline"));
8481
return internal;

‎modules/reindex/src/test/java/org/elasticsearch/reindex/RestDeleteByQueryActionTests.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
package org.elasticsearch.reindex;
1010

11-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1211
import org.elasticsearch.core.RestApiVersion;
1312
import org.elasticsearch.index.reindex.BulkByScrollResponse;
1413
import org.elasticsearch.rest.RestRequest;
@@ -31,7 +30,7 @@ public final class RestDeleteByQueryActionTests extends RestActionTestCase {
3130

3231
@Before
3332
public void setUpAction() {
34-
controller().registerHandler(new RestDeleteByQueryAction(mock(NamedWriteableRegistry.class), nf -> false));
33+
controller().registerHandler(new RestDeleteByQueryAction(nf -> false));
3534
verifyingClient.setExecuteVerifier((actionType, request) -> mock(BulkByScrollResponse.class));
3635
verifyingClient.setExecuteLocallyVerifier((actionType, request) -> mock(BulkByScrollResponse.class));
3736
}

‎modules/reindex/src/test/java/org/elasticsearch/reindex/RestReindexActionTests.java

+4-10
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import org.elasticsearch.common.bytes.BytesArray;
1212
import org.elasticsearch.common.bytes.BytesReference;
13-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1413
import org.elasticsearch.index.reindex.AbstractBulkByScrollRequest;
1514
import org.elasticsearch.index.reindex.ReindexRequest;
1615
import org.elasticsearch.test.rest.FakeRestRequest;
@@ -21,18 +20,16 @@
2120
import org.junit.Before;
2221

2322
import java.io.IOException;
24-
import java.util.Collections;
2523

2624
import static java.util.Collections.singletonMap;
27-
import static org.mockito.Mockito.mock;
2825

2926
public class RestReindexActionTests extends RestActionTestCase {
3027

3128
private RestReindexAction action;
3229

3330
@Before
3431
public void setUpAction() {
35-
action = new RestReindexAction(mock(NamedWriteableRegistry.class), nf -> false);
32+
action = new RestReindexAction(nf -> false);
3633
controller().registerHandler(action);
3734
}
3835

@@ -56,10 +53,7 @@ public void testPipelineQueryParameterIsError() throws IOException {
5653
request.withContent(BytesReference.bytes(body), body.contentType());
5754
}
5855
request.withParams(singletonMap("pipeline", "doesn't matter"));
59-
Exception e = expectThrows(
60-
IllegalArgumentException.class,
61-
() -> action.buildRequest(request.build(), new NamedWriteableRegistry(Collections.emptyList()))
62-
);
56+
Exception e = expectThrows(IllegalArgumentException.class, () -> action.buildRequest(request.build()));
6357

6458
assertEquals("_reindex doesn't support [pipeline] as a query parameter. Specify it in the [dest] object instead.", e.getMessage());
6559
}
@@ -68,14 +62,14 @@ public void testSetScrollTimeout() throws IOException {
6862
{
6963
FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry());
7064
requestBuilder.withContent(new BytesArray("{}"), XContentType.JSON);
71-
ReindexRequest request = action.buildRequest(requestBuilder.build(), new NamedWriteableRegistry(Collections.emptyList()));
65+
ReindexRequest request = action.buildRequest(requestBuilder.build());
7266
assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_TIMEOUT, request.getScrollTime());
7367
}
7468
{
7569
FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry());
7670
requestBuilder.withParams(singletonMap("scroll", "10m"));
7771
requestBuilder.withContent(new BytesArray("{}"), XContentType.JSON);
78-
ReindexRequest request = action.buildRequest(requestBuilder.build(), new NamedWriteableRegistry(Collections.emptyList()));
72+
ReindexRequest request = action.buildRequest(requestBuilder.build());
7973
assertEquals("10m", request.getScrollTime().toString());
8074
}
8175
}

‎modules/reindex/src/test/java/org/elasticsearch/reindex/RestUpdateByQueryActionTests.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
package org.elasticsearch.reindex;
1010

11-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1211
import org.elasticsearch.core.RestApiVersion;
1312
import org.elasticsearch.index.reindex.BulkByScrollResponse;
1413
import org.elasticsearch.rest.RestRequest;
@@ -31,7 +30,7 @@ public final class RestUpdateByQueryActionTests extends RestActionTestCase {
3130

3231
@Before
3332
public void setUpAction() {
34-
controller().registerHandler(new RestUpdateByQueryAction(mock(NamedWriteableRegistry.class), nf -> false));
33+
controller().registerHandler(new RestUpdateByQueryAction(nf -> false));
3534
verifyingClient.setExecuteVerifier((actionType, request) -> mock(BulkByScrollResponse.class));
3635
verifyingClient.setExecuteLocallyVerifier((actionType, request) -> mock(BulkByScrollResponse.class));
3736
}

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

+2-4
Original file line numberDiff line numberDiff line change
@@ -933,14 +933,12 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster, Predicate<
933933
registerHandler.accept(new RestBulkAction(settings));
934934
registerHandler.accept(new RestUpdateAction());
935935

936-
registerHandler.accept(new RestSearchAction(restController.getSearchUsageHolder(), namedWriteableRegistry, clusterSupportsFeature));
936+
registerHandler.accept(new RestSearchAction(restController.getSearchUsageHolder(), clusterSupportsFeature));
937937
registerHandler.accept(new RestSearchScrollAction());
938938
registerHandler.accept(new RestClearScrollAction());
939939
registerHandler.accept(new RestOpenPointInTimeAction());
940940
registerHandler.accept(new RestClosePointInTimeAction());
941-
registerHandler.accept(
942-
new RestMultiSearchAction(settings, restController.getSearchUsageHolder(), namedWriteableRegistry, clusterSupportsFeature)
943-
);
941+
registerHandler.accept(new RestMultiSearchAction(settings, restController.getSearchUsageHolder(), clusterSupportsFeature));
944942
registerHandler.accept(new RestKnnSearchAction());
945943

946944
registerHandler.accept(new RestValidateQueryAction());

0 commit comments

Comments
 (0)