mirror of
https://github.com/elastic/elasticsearch.git
synced 2025-04-24 15:17:30 -04:00
Clean up request/response for get-cluster-settings (#86347)
Today there are request and response objects for the get-cluster-settings action but the request is unused and the response is only used in the REST layer. This commit removes the unused request and renames the response to reflect that it's not a transport-layer response. It also tidies a few things up in this area, removing the unused `ActionResponse` superclass, making its fields final, and replacing the overly-general `RestBuilderListener` with a regular `RestToXContentListener` in the REST action. Relates #82342 because to resolve that issue we will want to introduce transport-layer request/response classes, and the classes involved in this commit are in the way of that change.
This commit is contained in:
David Turner
GitHub
parent
f0a86b5e38
commit
75ae3f8e6f
6 changed files with 39 additions and 97 deletions
|
|
@ -10,7 +10,7 @@ package org.elasticsearch.upgrades;
|
|||
|
||||
import org.apache.http.util.EntityUtils;
|
||||
import org.elasticsearch.Version;
|
||||
import org.elasticsearch.action.admin.cluster.settings.ClusterGetSettingsResponse;
|
||||
import org.elasticsearch.action.admin.cluster.settings.RestClusterGetSettingsResponse;
|
||||
import org.elasticsearch.client.Request;
|
||||
import org.elasticsearch.client.Response;
|
||||
import org.elasticsearch.client.ResponseException;
|
||||
|
|
@ -1849,8 +1849,7 @@ public class FullClusterRestartIT extends AbstractFullClusterRestartTestCase {
|
|||
final Request getSettingsRequest = new Request("GET", "/_cluster/settings");
|
||||
final Response getSettingsResponse = client().performRequest(getSettingsRequest);
|
||||
try (XContentParser parser = createParser(JsonXContent.jsonXContent, getSettingsResponse.getEntity().getContent())) {
|
||||
final ClusterGetSettingsResponse clusterGetSettingsResponse = ClusterGetSettingsResponse.fromXContent(parser);
|
||||
final Settings settings = clusterGetSettingsResponse.getPersistentSettings();
|
||||
final Settings settings = RestClusterGetSettingsResponse.fromXContent(parser).getPersistentSettings();
|
||||
assertThat(REMOTE_CLUSTER_COMPRESS.getConcreteSettingForNamespace("foo").get(settings), equalTo(Compression.Enabled.TRUE));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,37 +0,0 @@
|
|||
/*
|
||||
* 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.admin.cluster.settings;
|
||||
|
||||
import org.elasticsearch.action.ActionRequestValidationException;
|
||||
import org.elasticsearch.action.support.master.MasterNodeReadRequest;
|
||||
|
||||
/**
|
||||
* This request is specific to the REST client. {@link org.elasticsearch.action.admin.cluster.state.ClusterStateRequest}
|
||||
* is used on the transport layer.
|
||||
*/
|
||||
public class ClusterGetSettingsRequest extends MasterNodeReadRequest<ClusterGetSettingsRequest> {
|
||||
private boolean includeDefaults = false;
|
||||
|
||||
@Override
|
||||
public ActionRequestValidationException validate() {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* When include_defaults is set, return default settings which are normally suppressed.
|
||||
*/
|
||||
public ClusterGetSettingsRequest includeDefaults(boolean includeDefaults) {
|
||||
this.includeDefaults = includeDefaults;
|
||||
return this;
|
||||
}
|
||||
|
||||
public boolean includeDefaults() {
|
||||
return includeDefaults;
|
||||
}
|
||||
}
|
||||
|
|
@ -8,9 +8,7 @@
|
|||
|
||||
package org.elasticsearch.action.admin.cluster.settings;
|
||||
|
||||
import org.elasticsearch.action.ActionResponse;
|
||||
import org.elasticsearch.common.Strings;
|
||||
import org.elasticsearch.common.io.stream.StreamOutput;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.xcontent.ConstructingObjectParser;
|
||||
import org.elasticsearch.xcontent.ParseField;
|
||||
|
|
@ -28,22 +26,22 @@ import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstr
|
|||
* This response is specific to the REST client. {@link org.elasticsearch.action.admin.cluster.state.ClusterStateResponse}
|
||||
* is used on the transport layer.
|
||||
*/
|
||||
public class ClusterGetSettingsResponse extends ActionResponse implements ToXContentObject {
|
||||
public class RestClusterGetSettingsResponse implements ToXContentObject {
|
||||
|
||||
private Settings persistentSettings = Settings.EMPTY;
|
||||
private Settings transientSettings = Settings.EMPTY;
|
||||
private Settings defaultSettings = Settings.EMPTY;
|
||||
private final Settings persistentSettings;
|
||||
private final Settings transientSettings;
|
||||
private final Settings defaultSettings;
|
||||
|
||||
static final String PERSISTENT_FIELD = "persistent";
|
||||
static final String TRANSIENT_FIELD = "transient";
|
||||
static final String DEFAULTS_FIELD = "defaults";
|
||||
|
||||
private static final ConstructingObjectParser<ClusterGetSettingsResponse, Void> PARSER = new ConstructingObjectParser<>(
|
||||
private static final ConstructingObjectParser<RestClusterGetSettingsResponse, Void> PARSER = new ConstructingObjectParser<>(
|
||||
"cluster_get_settings_response",
|
||||
true,
|
||||
a -> {
|
||||
Settings defaultSettings = a[2] == null ? Settings.EMPTY : (Settings) a[2];
|
||||
return new ClusterGetSettingsResponse((Settings) a[0], (Settings) a[1], defaultSettings);
|
||||
return new RestClusterGetSettingsResponse((Settings) a[0], (Settings) a[1], defaultSettings);
|
||||
}
|
||||
);
|
||||
static {
|
||||
|
|
@ -52,16 +50,10 @@ public class ClusterGetSettingsResponse extends ActionResponse implements ToXCon
|
|||
PARSER.declareObject(optionalConstructorArg(), (p, c) -> Settings.fromXContent(p), new ParseField(DEFAULTS_FIELD));
|
||||
}
|
||||
|
||||
public ClusterGetSettingsResponse(Settings persistentSettings, Settings transientSettings, Settings defaultSettings) {
|
||||
if (persistentSettings != null) {
|
||||
this.persistentSettings = persistentSettings;
|
||||
}
|
||||
if (transientSettings != null) {
|
||||
this.transientSettings = transientSettings;
|
||||
}
|
||||
if (defaultSettings != null) {
|
||||
this.defaultSettings = defaultSettings;
|
||||
}
|
||||
public RestClusterGetSettingsResponse(Settings persistentSettings, Settings transientSettings, Settings defaultSettings) {
|
||||
this.persistentSettings = Objects.requireNonNullElse(persistentSettings, Settings.EMPTY);
|
||||
this.transientSettings = Objects.requireNonNullElse(transientSettings, Settings.EMPTY);
|
||||
this.defaultSettings = Objects.requireNonNullElse(defaultSettings, Settings.EMPTY);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -127,7 +119,7 @@ public class ClusterGetSettingsResponse extends ActionResponse implements ToXCon
|
|||
return builder;
|
||||
}
|
||||
|
||||
public static ClusterGetSettingsResponse fromXContent(XContentParser parser) {
|
||||
public static RestClusterGetSettingsResponse fromXContent(XContentParser parser) {
|
||||
return PARSER.apply(parser, null);
|
||||
}
|
||||
|
||||
|
|
@ -135,7 +127,7 @@ public class ClusterGetSettingsResponse extends ActionResponse implements ToXCon
|
|||
public boolean equals(Object o) {
|
||||
if (this == o) return true;
|
||||
if (o == null || getClass() != o.getClass()) return false;
|
||||
ClusterGetSettingsResponse that = (ClusterGetSettingsResponse) o;
|
||||
RestClusterGetSettingsResponse that = (RestClusterGetSettingsResponse) o;
|
||||
return Objects.equals(transientSettings, that.transientSettings)
|
||||
&& Objects.equals(persistentSettings, that.persistentSettings)
|
||||
&& Objects.equals(defaultSettings, that.defaultSettings);
|
||||
|
|
@ -150,7 +142,4 @@ public class ClusterGetSettingsResponse extends ActionResponse implements ToXCon
|
|||
public String toString() {
|
||||
return Strings.toString(this);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void writeTo(StreamOutput out) throws IOException {}
|
||||
}
|
||||
|
|
@ -8,9 +8,8 @@
|
|||
|
||||
package org.elasticsearch.rest.action.admin.cluster;
|
||||
|
||||
import org.elasticsearch.action.admin.cluster.settings.ClusterGetSettingsResponse;
|
||||
import org.elasticsearch.action.admin.cluster.settings.RestClusterGetSettingsResponse;
|
||||
import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest;
|
||||
import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
|
||||
import org.elasticsearch.client.internal.Requests;
|
||||
import org.elasticsearch.client.internal.node.NodeClient;
|
||||
import org.elasticsearch.cluster.ClusterState;
|
||||
|
|
@ -18,13 +17,8 @@ import org.elasticsearch.common.settings.ClusterSettings;
|
|||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.settings.SettingsFilter;
|
||||
import org.elasticsearch.rest.BaseRestHandler;
|
||||
import org.elasticsearch.rest.BytesRestResponse;
|
||||
import org.elasticsearch.rest.RestRequest;
|
||||
import org.elasticsearch.rest.RestResponse;
|
||||
import org.elasticsearch.rest.RestStatus;
|
||||
import org.elasticsearch.rest.action.RestBuilderListener;
|
||||
import org.elasticsearch.xcontent.ToXContent;
|
||||
import org.elasticsearch.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.rest.action.RestToXContentListener;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.List;
|
||||
|
|
@ -60,12 +54,14 @@ public class RestClusterGetSettingsAction extends BaseRestHandler {
|
|||
final boolean renderDefaults = request.paramAsBoolean("include_defaults", false);
|
||||
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
|
||||
clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
|
||||
return channel -> client.admin().cluster().state(clusterStateRequest, new RestBuilderListener<ClusterStateResponse>(channel) {
|
||||
@Override
|
||||
public RestResponse buildResponse(ClusterStateResponse response, XContentBuilder builder) throws Exception {
|
||||
return new BytesRestResponse(RestStatus.OK, renderResponse(response.getState(), renderDefaults, builder, request));
|
||||
}
|
||||
});
|
||||
return channel -> client.admin()
|
||||
.cluster()
|
||||
.state(
|
||||
clusterStateRequest,
|
||||
new RestToXContentListener<RestClusterGetSettingsResponse>(channel).map(
|
||||
response -> response(response.getState(), renderDefaults, settingsFilter, clusterSettings, settings)
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -78,19 +74,14 @@ public class RestClusterGetSettingsAction extends BaseRestHandler {
|
|||
return false;
|
||||
}
|
||||
|
||||
private XContentBuilder renderResponse(ClusterState state, boolean renderDefaults, XContentBuilder builder, ToXContent.Params params)
|
||||
throws IOException {
|
||||
return response(state, renderDefaults, settingsFilter, clusterSettings, settings).toXContent(builder, params);
|
||||
}
|
||||
|
||||
static ClusterGetSettingsResponse response(
|
||||
static RestClusterGetSettingsResponse response(
|
||||
final ClusterState state,
|
||||
final boolean renderDefaults,
|
||||
final SettingsFilter settingsFilter,
|
||||
final ClusterSettings clusterSettings,
|
||||
final Settings settings
|
||||
) {
|
||||
return new ClusterGetSettingsResponse(
|
||||
return new RestClusterGetSettingsResponse(
|
||||
settingsFilter.filter(state.metadata().persistentSettings()),
|
||||
settingsFilter.filter(state.metadata().transientSettings()),
|
||||
renderDefaults ? settingsFilter.filter(clusterSettings.diff(state.metadata().settings(), settings)) : Settings.EMPTY
|
||||
|
|
|
|||
|
|
@ -15,11 +15,11 @@ import org.elasticsearch.xcontent.XContentParser;
|
|||
import java.io.IOException;
|
||||
import java.util.function.Predicate;
|
||||
|
||||
public class ClusterGetSettingsResponseTests extends AbstractXContentTestCase<ClusterGetSettingsResponse> {
|
||||
public class RestClusterGetSettingsResponseTests extends AbstractXContentTestCase<RestClusterGetSettingsResponse> {
|
||||
|
||||
@Override
|
||||
protected ClusterGetSettingsResponse doParseInstance(XContentParser parser) throws IOException {
|
||||
return ClusterGetSettingsResponse.fromXContent(parser);
|
||||
protected RestClusterGetSettingsResponse doParseInstance(XContentParser parser) throws IOException {
|
||||
return RestClusterGetSettingsResponse.fromXContent(parser);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -28,17 +28,17 @@ public class ClusterGetSettingsResponseTests extends AbstractXContentTestCase<Cl
|
|||
}
|
||||
|
||||
@Override
|
||||
protected ClusterGetSettingsResponse createTestInstance() {
|
||||
protected RestClusterGetSettingsResponse createTestInstance() {
|
||||
Settings persistentSettings = ClusterUpdateSettingsResponseTests.randomClusterSettings(0, 2);
|
||||
Settings transientSettings = ClusterUpdateSettingsResponseTests.randomClusterSettings(0, 2);
|
||||
Settings defaultSettings = randomBoolean() ? ClusterUpdateSettingsResponseTests.randomClusterSettings(0, 2) : Settings.EMPTY;
|
||||
return new ClusterGetSettingsResponse(persistentSettings, transientSettings, defaultSettings);
|
||||
return new RestClusterGetSettingsResponse(persistentSettings, transientSettings, defaultSettings);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Predicate<String> getRandomFieldsExcludeFilter() {
|
||||
return p -> p.startsWith(ClusterGetSettingsResponse.TRANSIENT_FIELD)
|
||||
|| p.startsWith(ClusterGetSettingsResponse.PERSISTENT_FIELD)
|
||||
|| p.startsWith(ClusterGetSettingsResponse.DEFAULTS_FIELD);
|
||||
return p -> p.startsWith(RestClusterGetSettingsResponse.TRANSIENT_FIELD)
|
||||
|| p.startsWith(RestClusterGetSettingsResponse.PERSISTENT_FIELD)
|
||||
|| p.startsWith(RestClusterGetSettingsResponse.DEFAULTS_FIELD);
|
||||
}
|
||||
}
|
||||
|
|
@ -8,7 +8,7 @@
|
|||
|
||||
package org.elasticsearch.rest.action.admin.cluster;
|
||||
|
||||
import org.elasticsearch.action.admin.cluster.settings.ClusterGetSettingsResponse;
|
||||
import org.elasticsearch.action.admin.cluster.settings.RestClusterGetSettingsResponse;
|
||||
import org.elasticsearch.cluster.ClusterState;
|
||||
import org.elasticsearch.cluster.metadata.Metadata;
|
||||
import org.elasticsearch.common.settings.ClusterSettings;
|
||||
|
|
@ -27,16 +27,16 @@ import java.util.stream.Stream;
|
|||
public class RestClusterGetSettingsActionTests extends ESTestCase {
|
||||
|
||||
public void testFilterPersistentSettings() {
|
||||
runTestFilterSettingsTest(Metadata.Builder::persistentSettings, ClusterGetSettingsResponse::getPersistentSettings);
|
||||
runTestFilterSettingsTest(Metadata.Builder::persistentSettings, RestClusterGetSettingsResponse::getPersistentSettings);
|
||||
}
|
||||
|
||||
public void testFilterTransientSettings() {
|
||||
runTestFilterSettingsTest(Metadata.Builder::transientSettings, ClusterGetSettingsResponse::getTransientSettings);
|
||||
runTestFilterSettingsTest(Metadata.Builder::transientSettings, RestClusterGetSettingsResponse::getTransientSettings);
|
||||
}
|
||||
|
||||
private void runTestFilterSettingsTest(
|
||||
final BiConsumer<Metadata.Builder, Settings> md,
|
||||
final Function<ClusterGetSettingsResponse, Settings> s
|
||||
final Function<RestClusterGetSettingsResponse, Settings> s
|
||||
) {
|
||||
final Metadata.Builder mdBuilder = new Metadata.Builder();
|
||||
final Settings settings = Settings.builder().put("foo.filtered", "bar").put("foo.non_filtered", "baz").build();
|
||||
|
|
@ -52,7 +52,7 @@ public class RestClusterGetSettingsActionTests extends ESTestCase {
|
|||
)
|
||||
).collect(Collectors.toSet());
|
||||
final ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, settingsSet);
|
||||
final ClusterGetSettingsResponse response = RestClusterGetSettingsAction.response(
|
||||
final RestClusterGetSettingsResponse response = RestClusterGetSettingsAction.response(
|
||||
builder.build(),
|
||||
randomBoolean(),
|
||||
filter,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue