Skip to content

Commit f75ef7a

Browse files
nik9000nik9000
authored
Use namedObject to parse AllocationCommands (#22489)
This removes `AllocationCommandRegistry` entirely and replaces it with `XContentParser#namedObject`, removing another class from guice.
1 parent fc1f7c2 commit f75ef7a

File tree

7 files changed

+38
-91
lines changed

7 files changed

+38
-91
lines changed

‎core/src/main/java/org/elasticsearch/cluster/routing/allocation/command/AllocationCommandRegistry.java‎

Lines changed: 0 additions & 31 deletions
This file was deleted.

‎core/src/main/java/org/elasticsearch/cluster/routing/allocation/command/AllocationCommands.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,10 @@ public static void writeTo(AllocationCommands commands, StreamOutput out) throws
125125
* }
126126
* </pre>
127127
* @param parser {@link XContentParser} to read the commands from
128-
* @param registry of allocation command parsers
129128
* @return {@link AllocationCommands} read
130129
* @throws IOException if something bad happens while reading the stream
131130
*/
132-
public static AllocationCommands fromXContent(XContentParser parser, AllocationCommandRegistry registry) throws IOException {
131+
public static AllocationCommands fromXContent(XContentParser parser) throws IOException {
133132
AllocationCommands commands = new AllocationCommands();
134133

135134
XContentParser.Token token = parser.currentToken();
@@ -158,7 +157,7 @@ public static AllocationCommands fromXContent(XContentParser parser, AllocationC
158157
token = parser.nextToken();
159158
String commandName = parser.currentName();
160159
token = parser.nextToken();
161-
commands.add(registry.lookup(commandName, parser.getTokenLocation()).fromXContent(parser));
160+
commands.add(parser.namedObject(AllocationCommand.class, commandName, null));
162161
// move to the end object one
163162
if (parser.nextToken() != XContentParser.Token.END_OBJECT) {
164163
throw new ElasticsearchParseException("allocation command is malformed, done parsing a command, but didn't get END_OBJECT, got [{}] instead", token);

‎core/src/main/java/org/elasticsearch/common/network/NetworkModule.java‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,17 @@
2424
import org.elasticsearch.cluster.routing.allocation.command.AllocateReplicaAllocationCommand;
2525
import org.elasticsearch.cluster.routing.allocation.command.AllocateStalePrimaryAllocationCommand;
2626
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommand;
27-
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommandRegistry;
2827
import org.elasticsearch.cluster.routing.allocation.command.CancelAllocationCommand;
2928
import org.elasticsearch.cluster.routing.allocation.command.MoveAllocationCommand;
3029
import org.elasticsearch.common.ParseField;
3130
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
32-
import org.elasticsearch.common.io.stream.NamedWriteableRegistry.Entry;
3331
import org.elasticsearch.common.io.stream.Writeable;
3432
import org.elasticsearch.common.settings.Setting;
3533
import org.elasticsearch.common.settings.Setting.Property;
3634
import org.elasticsearch.common.settings.Settings;
3735
import org.elasticsearch.common.util.BigArrays;
3836
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
37+
import org.elasticsearch.common.xcontent.NamedXContentRegistry.FromXContent;
3938
import org.elasticsearch.http.HttpServerTransport;
4039
import org.elasticsearch.indices.breaker.CircuitBreakerService;
4140
import org.elasticsearch.plugins.NetworkPlugin;
@@ -76,12 +75,8 @@ public final class NetworkModule {
7675
private final Settings settings;
7776
private final boolean transportClient;
7877

79-
private static final AllocationCommandRegistry allocationCommandRegistry = new AllocationCommandRegistry();
8078
private static final List<NamedWriteableRegistry.Entry> namedWriteables = new ArrayList<>();
81-
82-
private final Map<String, Supplier<Transport>> transportFactories = new HashMap<>();
83-
private final Map<String, Supplier<HttpServerTransport>> transportHttpFactories = new HashMap<>();
84-
private final List<TransportInterceptor> transportIntercetors = new ArrayList<>();
79+
private static final List<NamedXContentRegistry.Entry> namedXContents = new ArrayList<>();
8580

8681
static {
8782
registerAllocationCommand(CancelAllocationCommand::new, CancelAllocationCommand::fromXContent,
@@ -99,6 +94,11 @@ public final class NetworkModule {
9994
namedWriteables.add(
10095
new NamedWriteableRegistry.Entry(Task.Status.class, RawTaskStatus.NAME, RawTaskStatus::new));
10196
}
97+
98+
private final Map<String, Supplier<Transport>> transportFactories = new HashMap<>();
99+
private final Map<String, Supplier<HttpServerTransport>> transportHttpFactories = new HashMap<>();
100+
private final List<TransportInterceptor> transportIntercetors = new ArrayList<>();
101+
102102
/**
103103
* Creates a network module that custom networking classes can be plugged into.
104104
* @param settings The settings for the node
@@ -165,20 +165,17 @@ private void registerHttpTransport(String key, Supplier<HttpServerTransport> fac
165165
* it is the name under which the command's reader is registered.
166166
*/
167167
private static <T extends AllocationCommand> void registerAllocationCommand(Writeable.Reader<T> reader,
168-
AllocationCommand.Parser<T> parser, ParseField commandName) {
169-
allocationCommandRegistry.register(parser, commandName);
170-
namedWriteables.add(new Entry(AllocationCommand.class, commandName.getPreferredName(), reader));
168+
FromXContent<T> parser, ParseField commandName) {
169+
namedXContents.add(new NamedXContentRegistry.Entry(AllocationCommand.class, commandName, parser));
170+
namedWriteables.add(new NamedWriteableRegistry.Entry(AllocationCommand.class, commandName.getPreferredName(), reader));
171171
}
172172

173-
/**
174-
* The registry of allocation command parsers.
175-
*/
176-
public static AllocationCommandRegistry getAllocationCommandRegistry() {
177-
return allocationCommandRegistry;
173+
public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
174+
return Collections.unmodifiableList(namedWriteables);
178175
}
179176

180-
public static List<Entry> getNamedWriteables() {
181-
return Collections.unmodifiableList(namedWriteables);
177+
public static List<NamedXContentRegistry.Entry> getNamedXContents() {
178+
return Collections.unmodifiableList(namedXContents);
182179
}
183180

184181
public Supplier<HttpServerTransport> getHttpServerTransportSupplier() {

‎core/src/main/java/org/elasticsearch/node/Node.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
import org.elasticsearch.cluster.node.DiscoveryNode;
4848
import org.elasticsearch.cluster.routing.RoutingService;
4949
import org.elasticsearch.cluster.routing.allocation.AllocationService;
50-
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommandRegistry;
5150
import org.elasticsearch.cluster.service.ClusterService;
5251
import org.elasticsearch.common.StopWatch;
5352
import org.elasticsearch.common.component.Lifecycle;
@@ -361,6 +360,7 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
361360
.flatMap(Function.identity()).collect(Collectors.toList());
362361
final NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(namedWriteables);
363362
NamedXContentRegistry xContentRegistry = new NamedXContentRegistry(Stream.of(
363+
NetworkModule.getNamedXContents().stream(),
364364
searchModule.getNamedXContents().stream(),
365365
pluginsService.filterPlugins(Plugin.class).stream()
366366
.flatMap(p -> p.getNamedXContent().stream()),
@@ -437,7 +437,6 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
437437
b.bind(Transport.class).toInstance(transport);
438438
b.bind(TransportService.class).toInstance(transportService);
439439
b.bind(NetworkService.class).toInstance(networkService);
440-
b.bind(AllocationCommandRegistry.class).toInstance(NetworkModule.getAllocationCommandRegistry());
441440
b.bind(UpdateHelper.class).toInstance(new UpdateHelper(settings, scriptModule.getScriptService()));
442441
b.bind(MetaDataIndexUpgradeService.class).toInstance(new MetaDataIndexUpgradeService(settings,
443442
xContentRegistry, indicesModule.getMapperRegistry(), settingsModule.getIndexScopedSettings()));

‎core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterRerouteAction.java‎

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,8 @@
2424
import org.elasticsearch.client.Requests;
2525
import org.elasticsearch.client.node.NodeClient;
2626
import org.elasticsearch.cluster.ClusterState;
27-
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommandRegistry;
2827
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommands;
2928
import org.elasticsearch.common.ParseField;
30-
import org.elasticsearch.common.ParseFieldMatcher;
31-
import org.elasticsearch.common.ParseFieldMatcherSupplier;
3229
import org.elasticsearch.common.Strings;
3330
import org.elasticsearch.common.inject.Inject;
3431
import org.elasticsearch.common.settings.Settings;
@@ -49,31 +46,28 @@
4946
import java.util.Set;
5047

5148
public class RestClusterRerouteAction extends BaseRestHandler {
52-
private static final ObjectParser<ClusterRerouteRequest, ParseContext> PARSER = new ObjectParser<>("cluster_reroute");
49+
private static final ObjectParser<ClusterRerouteRequest, Void> PARSER = new ObjectParser<>("cluster_reroute");
5350
static {
54-
PARSER.declareField((p, v, c) -> v.commands(AllocationCommands.fromXContent(p, c.registry)),
55-
new ParseField("commands"), ValueType.OBJECT_ARRAY);
51+
PARSER.declareField((p, v, c) -> v.commands(AllocationCommands.fromXContent(p)), new ParseField("commands"),
52+
ValueType.OBJECT_ARRAY);
5653
PARSER.declareBoolean(ClusterRerouteRequest::dryRun, new ParseField("dry_run"));
5754
}
5855

5956
private static final String DEFAULT_METRICS = Strings
6057
.arrayToCommaDelimitedString(EnumSet.complementOf(EnumSet.of(ClusterState.Metric.METADATA)).toArray());
6158

6259
private final SettingsFilter settingsFilter;
63-
private final AllocationCommandRegistry registry;
6460

6561
@Inject
66-
public RestClusterRerouteAction(Settings settings, RestController controller, SettingsFilter settingsFilter,
67-
AllocationCommandRegistry registry) {
62+
public RestClusterRerouteAction(Settings settings, RestController controller, SettingsFilter settingsFilter) {
6863
super(settings);
6964
this.settingsFilter = settingsFilter;
70-
this.registry = registry;
7165
controller.registerHandler(RestRequest.Method.POST, "/_cluster/reroute", this);
7266
}
7367

7468
@Override
7569
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
76-
ClusterRerouteRequest clusterRerouteRequest = createRequest(request, registry, parseFieldMatcher);
70+
ClusterRerouteRequest clusterRerouteRequest = createRequest(request);
7771

7872
// by default, return everything but metadata
7973
final String metric = request.param("metric");
@@ -111,31 +105,14 @@ protected Set<String> responseParams() {
111105
return RESPONSE_PARAMS;
112106
}
113107

114-
public static ClusterRerouteRequest createRequest(RestRequest request, AllocationCommandRegistry registry,
115-
ParseFieldMatcher parseFieldMatcher) throws IOException {
108+
public static ClusterRerouteRequest createRequest(RestRequest request) throws IOException {
116109
ClusterRerouteRequest clusterRerouteRequest = Requests.clusterRerouteRequest();
117110
clusterRerouteRequest.dryRun(request.paramAsBoolean("dry_run", clusterRerouteRequest.dryRun()));
118111
clusterRerouteRequest.explain(request.paramAsBoolean("explain", clusterRerouteRequest.explain()));
119112
clusterRerouteRequest.timeout(request.paramAsTime("timeout", clusterRerouteRequest.timeout()));
120113
clusterRerouteRequest.setRetryFailed(request.paramAsBoolean("retry_failed", clusterRerouteRequest.isRetryFailed()));
121114
clusterRerouteRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterRerouteRequest.masterNodeTimeout()));
122-
request.applyContentParser(parser -> PARSER.parse(parser, clusterRerouteRequest, new ParseContext(registry, parseFieldMatcher)));
115+
request.applyContentParser(parser -> PARSER.parse(parser, clusterRerouteRequest, null));
123116
return clusterRerouteRequest;
124117
}
125-
126-
private static class ParseContext implements ParseFieldMatcherSupplier {
127-
private final AllocationCommandRegistry registry;
128-
private final ParseFieldMatcher parseFieldMatcher;
129-
130-
private ParseContext(AllocationCommandRegistry registry, ParseFieldMatcher parseFieldMatcher) {
131-
this.registry = registry;
132-
this.parseFieldMatcher = parseFieldMatcher;
133-
}
134-
135-
@Override
136-
public ParseFieldMatcher getParseFieldMatcher() {
137-
return parseFieldMatcher;
138-
}
139-
}
140-
141118
}

‎core/src/test/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteRequestTests.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,14 @@
2525
import org.elasticsearch.cluster.routing.allocation.command.AllocateReplicaAllocationCommand;
2626
import org.elasticsearch.cluster.routing.allocation.command.AllocateStalePrimaryAllocationCommand;
2727
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommand;
28-
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommandRegistry;
2928
import org.elasticsearch.cluster.routing.allocation.command.CancelAllocationCommand;
3029
import org.elasticsearch.cluster.routing.allocation.command.MoveAllocationCommand;
31-
import org.elasticsearch.common.ParseFieldMatcher;
3230
import org.elasticsearch.common.io.stream.BytesStreamOutput;
3331
import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
3432
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
3533
import org.elasticsearch.common.io.stream.StreamInput;
3634
import org.elasticsearch.common.network.NetworkModule;
35+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
3736
import org.elasticsearch.common.xcontent.ToXContent;
3837
import org.elasticsearch.common.xcontent.XContentBuilder;
3938
import org.elasticsearch.common.xcontent.XContentFactory;
@@ -71,10 +70,8 @@ Arrays.<Supplier<AllocationCommand>> asList(
7170
() -> new MoveAllocationCommand(randomAsciiOfLengthBetween(2, 10), between(0, 1000),
7271
randomAsciiOfLengthBetween(2, 10), randomAsciiOfLengthBetween(2, 10))));
7372
private final NamedWriteableRegistry namedWriteableRegistry;
74-
private final AllocationCommandRegistry allocationCommandRegistry;
7573

7674
public ClusterRerouteRequestTests() {
77-
allocationCommandRegistry = NetworkModule.getAllocationCommandRegistry();
7875
namedWriteableRegistry = new NamedWriteableRegistry(NetworkModule.getNamedWriteables());
7976
}
8077

@@ -176,7 +173,7 @@ private ClusterRerouteRequest roundTripThroughBytes(ClusterRerouteRequest origin
176173

177174
private ClusterRerouteRequest roundTripThroughRestRequest(ClusterRerouteRequest original) throws IOException {
178175
RestRequest restRequest = toRestRequest(original);
179-
return RestClusterRerouteAction.createRequest(restRequest, allocationCommandRegistry, ParseFieldMatcher.STRICT);
176+
return RestClusterRerouteAction.createRequest(restRequest);
180177
}
181178

182179
private RestRequest toRestRequest(ClusterRerouteRequest original) throws IOException {
@@ -216,4 +213,9 @@ private RestRequest toRestRequest(ClusterRerouteRequest original) throws IOExcep
216213
}
217214
return requestBuilder.build();
218215
}
216+
217+
@Override
218+
protected NamedXContentRegistry xContentRegistry() {
219+
return new NamedXContentRegistry(NetworkModule.getNamedXContents());
220+
}
219221
}

‎core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationCommandsTests.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import org.elasticsearch.cluster.routing.allocation.command.AllocateEmptyPrimaryAllocationCommand;
3434
import org.elasticsearch.cluster.routing.allocation.command.AllocateReplicaAllocationCommand;
3535
import org.elasticsearch.cluster.routing.allocation.command.AllocateStalePrimaryAllocationCommand;
36-
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommandRegistry;
3736
import org.elasticsearch.cluster.routing.allocation.command.AllocationCommands;
3837
import org.elasticsearch.cluster.routing.allocation.command.CancelAllocationCommand;
3938
import org.elasticsearch.cluster.routing.allocation.command.MoveAllocationCommand;
@@ -45,6 +44,7 @@
4544
import org.elasticsearch.common.logging.Loggers;
4645
import org.elasticsearch.common.network.NetworkModule;
4746
import org.elasticsearch.common.settings.Settings;
47+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
4848
import org.elasticsearch.common.xcontent.XContentParser;
4949
import org.elasticsearch.common.xcontent.json.JsonXContent;
5050
import org.elasticsearch.index.IndexNotFoundException;
@@ -488,8 +488,7 @@ public void testXContent() throws Exception {
488488
// move two tokens, parser expected to be "on" `commands` field
489489
parser.nextToken();
490490
parser.nextToken();
491-
AllocationCommandRegistry registry = NetworkModule.getAllocationCommandRegistry();
492-
AllocationCommands sCommands = AllocationCommands.fromXContent(parser, registry);
491+
AllocationCommands sCommands = AllocationCommands.fromXContent(parser);
493492

494493
assertThat(sCommands.commands().size(), equalTo(5));
495494
assertThat(((AllocateEmptyPrimaryAllocationCommand) (sCommands.commands().get(0))).shardId(), equalTo(1));
@@ -516,4 +515,9 @@ public void testXContent() throws Exception {
516515
assertThat(((CancelAllocationCommand) (sCommands.commands().get(4))).node(), equalTo("node5"));
517516
assertThat(((CancelAllocationCommand) (sCommands.commands().get(4))).allowPrimary(), equalTo(true));
518517
}
518+
519+
@Override
520+
protected NamedXContentRegistry xContentRegistry() {
521+
return new NamedXContentRegistry(NetworkModule.getNamedXContents());
522+
}
519523
}

0 commit comments

Comments
 (0)