From a0210d47accea3fe8e439d7eb037847f5deabfbb Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Fri, 23 Mar 2018 14:25:46 -0400 Subject: [PATCH 01/11] Add Get Settings API support to java high-level rest client --- .../elasticsearch/client/IndicesClient.java | 24 +++++ .../org/elasticsearch/client/Request.java | 13 +++ .../elasticsearch/client/IndicesClientIT.java | 62 +++++++++++++ .../elasticsearch/client/RequestTests.java | 24 +++++ .../IndicesClientDocumentationIT.java | 73 +++++++++++++++- .../high-level/indices/get_settings.asciidoc | 87 +++++++++++++++++++ .../high-level/supported-apis.asciidoc | 2 + .../api/indices.get_settings.json | 4 + .../settings/get/GetSettingsResponse.java | 73 +++++++++++++++- .../get/GetSettingsResponseTests.java | 39 +++++++++ 10 files changed, 399 insertions(+), 2 deletions(-) create mode 100644 docs/java-rest/high-level/indices/get_settings.asciidoc create mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java index ff9c612e1d475..6202e14a97201 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java @@ -43,6 +43,8 @@ import org.elasticsearch.action.admin.indices.open.OpenIndexResponse; import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.admin.indices.refresh.RefreshResponse; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsRequest; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; import org.elasticsearch.action.admin.indices.rollover.RolloverResponse; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; @@ -265,6 +267,28 @@ public void flushAsync(FlushRequest flushRequest, ActionListener listener, emptySet(), headers); } + /** + * Retrieve the settings of one or more indices + *

+ * See + * Indices Get Settings API on elastic.co + */ + public GetSettingsResponse getSettings(GetSettingsRequest getSettingsRequest, Header... headers) throws IOException { + return restHighLevelClient.performRequestAndParseEntity(getSettingsRequest, Request::getSettings, GetSettingsResponse::fromXContent, + emptySet(), headers); + } + + /** + * Asynchronously retrieve the settings of one or more indices + *

+ * See + * Indices Get Settings API on elastic.co + */ + public void getSettingsAsync(GetSettingsRequest getSettingsRequest, ActionListener listener, Header... headers) { + restHighLevelClient.performRequestAsyncAndParseEntity(getSettingsRequest, Request::getSettings, GetSettingsResponse::fromXContent, + listener, emptySet(), headers); + } + /** * Force merge one or more indices using the Force Merge API *

diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java index 4e6fcdbb8dd4a..85e760f7eef93 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java @@ -44,6 +44,7 @@ import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeType; import org.elasticsearch.action.bulk.BulkRequest; @@ -593,6 +594,18 @@ static Request rollover(RolloverRequest rolloverRequest) throws IOException { return new Request(HttpPost.METHOD_NAME, endpoint, params.getParams(), entity); } + static Request getSettings(GetSettingsRequest getSettingsRequest) throws IOException { + Params params = Params.builder(); + params.withIndicesOptions(getSettingsRequest.indicesOptions()); + params.withLocal(getSettingsRequest.local()); + + if (getSettingsRequest.indices().length == 0) { + throw new IllegalArgumentException("getSettings requires at least one index"); + } + String endpoint = endpoint(getSettingsRequest.indices(), "_settings", getSettingsRequest.names()); + return new Request(HttpGet.METHOD_NAME, endpoint, params.getParams(), null); + } + static Request indicesExist(GetIndexRequest request) { // this can be called with no indices as argument by transport client, not via REST though if (request.indices() == null || request.indices().length == 0) { diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java index 0feb78d66b2dd..ecb1d7f40fe19 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java @@ -51,6 +51,8 @@ import org.elasticsearch.action.admin.indices.rollover.RolloverResponse; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsResponse; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsRequest; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; import org.elasticsearch.action.admin.indices.shrink.ResizeRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeResponse; import org.elasticsearch.action.admin.indices.shrink.ResizeType; @@ -189,6 +191,66 @@ public void testCreateIndex() throws IOException { } } + public void testGetSettings() throws IOException { + String indexName = "get_settings_index"; + Settings basicSettings = Settings.builder() + .put("number_of_shards", 13) + .put("number_of_replicas", 0) + .build(); + createIndex(indexName, basicSettings); + + GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indexName); + GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync); + + assertNull(getSettingsResponse.getSetting(indexName, "index.refresh_interval")); + assertEquals("13", getSettingsResponse.getSetting(indexName, "index.number_of_shards")); + + updateIndexSettings(indexName, Settings.builder().put("refresh_interval", "30s")); + + GetSettingsResponse updatedResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync); + assertEquals("30s", updatedResponse.getSetting(indexName, "index.refresh_interval")); + } + + public void testGetSettingsNonExistentIndex() throws IOException { + String nonExistentIndex = "index_that_doesnt_exist"; + assertFalse(indexExists(nonExistentIndex)); + + GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(nonExistentIndex); + ElasticsearchException exception = expectThrows(ElasticsearchException.class, + () -> execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync)); + assertEquals(RestStatus.NOT_FOUND, exception.status()); + } + + public void testGetMultipleSettings() throws IOException { + String indexName1 = "get_multiple_settings_one"; + createIndex(indexName1, Settings.builder().put("number_of_shards", 11).build()); + + String indexName2 = "get_multiple_settings_two"; + createIndex(indexName2, Settings.builder().put("number_of_shards", 22).build()); + + GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices("get_multiple_settings*"); + GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync); + + assertEquals("11", getSettingsResponse.getSetting(indexName1, "index.number_of_shards")); + assertEquals("22", getSettingsResponse.getSetting(indexName2, "index.number_of_shards")); + } + + public void testGetSettingsFiltered() throws IOException { + String indexName = "get_settings_index"; + Settings basicSettings = Settings.builder() + .put("number_of_shards", 13) + .put("number_of_replicas", 0) + .build(); + createIndex(indexName, basicSettings); + + GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indexName).names("index.number_of_shards"); + GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync); + + assertNull(getSettingsResponse.getSetting(indexName, "index.number_of_replicas")); + assertEquals("13", getSettingsResponse.getSetting(indexName, "index.number_of_shards")); + assertEquals(1, getSettingsResponse.getIndexToSettings().get("get_settings_index").size()); + } + public void testPutMapping() throws IOException { { // Add mappings to index diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index abce180546dfc..3e02639ac9e41 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -47,6 +47,7 @@ import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeType; import org.elasticsearch.action.bulk.BulkRequest; @@ -428,6 +429,29 @@ public void testDeleteIndex() { assertNull(request.getEntity()); } + public void testGetSettings() throws IOException { + String[] indices = randomIndicesNames(1, 5); + GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indices); + + Map expectedParams = new HashMap<>(); + /* HELP + setRandomMasterTimeout(getSettingsRequest, expectedParams); + I definitely have some questions here. Enabling this causes the master timeout + parameter *not* to come through on the actual request, but I can't figure out + why. Further, I'm not entirely sure I need it. + */ + setRandomIndicesOptions(getSettingsRequest::indicesOptions, getSettingsRequest::indicesOptions, expectedParams); + + setRandomLocal(getSettingsRequest, expectedParams); + + Request request = Request.getSettings(getSettingsRequest); + StringJoiner endpoint = new StringJoiner("/", "/", "").add(String.join(",", indices)).add("_settings"); + assertThat(endpoint.toString(), equalTo(request.getEndpoint())); + assertThat(request.getParameters(), equalTo(expectedParams)); + assertThat(request.getMethod(), equalTo(HttpGet.METHOD_NAME)); + assertThat(request.getEntity(), nullValue()); + } + public void testDeleteIndexEmptyIndices() { String[] indices = randomBoolean() ? null : Strings.EMPTY_ARRAY; ActionRequestValidationException validationException = new DeleteIndexRequest(indices).validate(); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java index e33d1e4729b0e..8a0989d27ea39 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java @@ -50,6 +50,8 @@ import org.elasticsearch.action.admin.indices.rollover.RolloverResponse; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsResponse; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsRequest; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; import org.elasticsearch.action.admin.indices.shrink.ResizeRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeResponse; import org.elasticsearch.action.admin.indices.shrink.ResizeType; @@ -58,7 +60,6 @@ import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.ESRestHighLevelClientTestCase; import org.elasticsearch.client.RestHighLevelClient; -import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; @@ -775,6 +776,76 @@ public void onFailure(Exception e) { } // end::flush-notfound } + } + + public void testGetSettings() throws Exception { + RestHighLevelClient client = highLevelClient(); + + { + Settings settings = Settings.builder().put("number_of_shards", 3).build(); + CreateIndexResponse createIndexResponse = client.indices().create(new CreateIndexRequest("index", settings)); + assertTrue(createIndexResponse.isAcknowledged()); + } + + // tag::get-settings-request + GetSettingsRequest request = new GetSettingsRequest().indices("index"); + // tag::get-settings-request + + // tag::get-settings-request-names + request.names("index.number_of_shards"); // <1> + // end::get-settings-request-names + + // tag::get-settings-request-masterTimeout + request.masterNodeTimeout(TimeValue.timeValueMinutes(1)); // <1> + request.masterNodeTimeout("1m"); // <2> + // end::get-settings-request-masterTimeout + + // tag::get-settings-request-indicesOptions + request.indicesOptions(IndicesOptions.lenientExpandOpen()); // <1> + // end::get-settings-request-indicesOptions + + + // tag::get-settings-execute + GetSettingsResponse getSettingsResponse = client.indices().getSettings(request); + // end::get-settings-execute + + // tag::get-settings-response + String numberOfShardsString = getSettingsResponse.getSetting("index", "index.number_of_shards"); // <1> + Settings indexSettings = getSettingsResponse.getIndexToSettings().get("index"); // <2> + int numberOfShards = indexSettings.getAsInt("index.number_of_shards", null); // <3> + // end::get-settings-response + + assertEquals("3", numberOfShardsString); + assertEquals(3, numberOfShards); + + // tag::get-settings-execute-listener + ActionListener listener = + new ActionListener() { + @Override + public void onResponse(GetSettingsResponse GetSettingsResponse) { + // <1> + } + + @Override + public void onFailure(Exception e) { + // <2> + } + }; + // end::get-settings-execute-listener + + // Replace the empty listener by a blocking listener in test + final CountDownLatch latch = new CountDownLatch(1); + listener = new LatchedActionListener<>(listener, latch); + + // tag::get-settings-execute-async + client.indices().getSettingsAsync(request, listener); // <1> + // end::get-settings-execute-async + + assertTrue(latch.await(30L, TimeUnit.SECONDS)); + + + + } public void testForceMergeIndex() throws Exception { diff --git a/docs/java-rest/high-level/indices/get_settings.asciidoc b/docs/java-rest/high-level/indices/get_settings.asciidoc new file mode 100644 index 0000000000000..c4bf1abb61b83 --- /dev/null +++ b/docs/java-rest/high-level/indices/get_settings.asciidoc @@ -0,0 +1,87 @@ +[[java-rest-high-get-settings]] +=== Get Settings API + +[[java-rest-high-get-settings-request]] +==== Get Settings Request + +A `GetSettingsRequest` requires one or more `index` arguments: + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request] +-------------------------------------------------- +<1> The index whose settings we should retrieve + +==== Optional arguments +The following arguments can optionally be provided: + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request-names] +-------------------------------------------------- +<1> One or more settings that be the only settings retrieved. If unset, all settings will be retrieved + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request-masterTimeout] +-------------------------------------------------- +<1> Timeout to connect to the master node as a `TimeValue` +<2> Timeout to connect to the master node as a `String` + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request-indicesOptions] +-------------------------------------------------- +<1> Setting `IndicesOptions` controls how unavailable indices are resolved and +how wildcard expressions are expanded + +[[java-rest-high-get-settings-sync]] +==== Synchronous Execution + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-execute] +-------------------------------------------------- + +[[java-rest-high-get-settings-async]] +==== Asynchronous Execution + +The asynchronous execution of a Get Settings request requires both the `GetSettingsRequest` +instance and an `ActionListener` instance to be passed to the asynchronous +method: + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-execute-async] +-------------------------------------------------- +<1> The `GetSettingsRequest` to execute and the `ActionListener` to use when +the execution completes + +The asynchronous method does not block and returns immediately. Once it is +completed the `ActionListener` is called back using the `onResponse` method +if the execution successfully completed or using the `onFailure` method if +it failed. + +A typical listener for `GetSettingsResponse` looks like: + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-execute-listener] +-------------------------------------------------- +<1> Called when the execution is successfully completed. The response is +provided as an argument +<2> Called in case of failure. The raised exception is provided as an argument + +[[java-rest-high-get-settings-response]] +==== Get Settings Response + +The returned `GetSettingsResponse` allows to retrieve information about the +executed operation as follows: + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-response] +-------------------------------------------------- +<1> We can retrieve the setting value for a particular index directly from the response as a string +<2> We can also retrieve the Settings object for a particular index for further examination +<3> The returned Settings object provides convenience methods for non String types diff --git a/docs/java-rest/high-level/supported-apis.asciidoc b/docs/java-rest/high-level/supported-apis.asciidoc index 29052171cddc6..7e0b5ce4f4cad 100644 --- a/docs/java-rest/high-level/supported-apis.asciidoc +++ b/docs/java-rest/high-level/supported-apis.asciidoc @@ -65,6 +65,7 @@ Index Management:: * <> * <> * <> +* <> Mapping Management:: * <> @@ -89,6 +90,7 @@ include::indices/put_mapping.asciidoc[] include::indices/update_aliases.asciidoc[] include::indices/exists_alias.asciidoc[] include::indices/put_settings.asciidoc[] +include::indices/get_settings.asciidoc[] == Cluster APIs diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_settings.json b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_settings.json index 706cce5277a40..ed22cc837d6a8 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_settings.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_settings.json @@ -16,6 +16,10 @@ } }, "params": { + "master_timeout": { + "type": "time", + "description": "Specify timeout for connection to master" + }, "ignore_unavailable": { "type" : "boolean", "description" : "Whether specified concrete indices should be ignored when unavailable (missing or closed)" diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java index 0a3229dcaf1b3..790a35f743e0a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java @@ -21,17 +21,27 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsResponse; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.*; import java.io.IOException; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.Objects; -public class GetSettingsResponse extends ActionResponse { +public class GetSettingsResponse extends ActionResponse implements ToXContentObject { private ImmutableOpenMap indexToSettings = ImmutableOpenMap.of(); + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "get_index_settings_response", true, args -> new GetSettingsResponse() + ); + public GetSettingsResponse(ImmutableOpenMap indexToSettings) { this.indexToSettings = indexToSettings; } @@ -72,4 +82,65 @@ public void writeTo(StreamOutput out) throws IOException { Settings.writeSettingsToStream(cursor.value, out); } } + + public static GetSettingsResponse fromXContent(XContentParser parser) throws IOException { + HashMap indexToSettings = new HashMap<>(); + if (parser.currentToken() == null) { + parser.nextToken(); + } + XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); + parser.nextToken(); + + while (!parser.isClosed() && parser.currentToken() == XContentParser.Token.FIELD_NAME) { + String indexName = parser.currentName(); + parser.nextToken(); //go to per-index settings object + XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); + XContentParserUtils.ensureFieldName(parser, parser.nextToken(), "settings"); + parser.nextToken(); //go to settings object itself + XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); + Settings settings = Settings.fromXContent(parser); + parser.nextToken(); //end per-index object + XContentParserUtils.ensureExpectedToken(XContentParser.Token.END_OBJECT, parser.currentToken(), parser::getTokenLocation); + parser.nextToken(); //we should now be positioned at the beginning of the next index's settings, or at the end + indexToSettings.put(indexName, settings); + } + return new GetSettingsResponse(ImmutableOpenMap.builder().putAll(indexToSettings).build()); + } + @Override + public String toString() { + return indexToSettings.toString(); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + Iterator indicesIterator = indexToSettings.keysIt(); + while (indicesIterator.hasNext()) { + String indexName = indicesIterator.next(); + Settings settings = indexToSettings.get(indexName); + builder.field(indexName); + builder.startObject(); + builder.field("settings"); + builder.startObject(); + settings.toXContent(builder, params); + builder.endObject(); + builder.endObject(); + } + builder.endObject(); + return builder; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + GetSettingsResponse that = (GetSettingsResponse) o; + return Objects.equals(indexToSettings, that.indexToSettings); + } + + @Override + public int hashCode() { + + return Objects.hash(indexToSettings); + } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java new file mode 100644 index 0000000000000..173f21d29d084 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java @@ -0,0 +1,39 @@ +package org.elasticsearch.action.admin.indices.settings.get; + +import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.index.RandomCreateIndexGenerator; +import org.elasticsearch.test.AbstractStreamableXContentTestCase; + +import java.io.IOException; +import java.util.HashMap; + +public class GetSettingsResponseTests extends AbstractStreamableXContentTestCase { + @Override + protected GetSettingsResponse createBlankInstance() { + return new GetSettingsResponse(); + } + + @Override + protected GetSettingsResponse createTestInstance() { + HashMap indexToSettings = new HashMap<>(); + int numIndices = randomIntBetween(1, 5); + for (int x=0;x immutableIndexToSettings = ImmutableOpenMap.builder().putAll(indexToSettings).build(); + return new GetSettingsResponse(immutableIndexToSettings); + } + + @Override + protected GetSettingsResponse doParseInstance(XContentParser parser) throws IOException { + return GetSettingsResponse.fromXContent(parser); + } + + @Override + protected boolean supportsUnknownFields() { + return false; + } +} From 74185c105852dc75c30f2c7f6774627b10e82655 Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Fri, 23 Mar 2018 16:52:32 -0400 Subject: [PATCH 02/11] Fix bad import style in GetSettingsResponse.java --- .../admin/indices/settings/get/GetSettingsResponse.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java index 790a35f743e0a..c647adc758fa0 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java @@ -21,17 +21,19 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsResponse; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.*; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentParserUtils; import java.io.IOException; import java.util.HashMap; import java.util.Iterator; -import java.util.Map; import java.util.Objects; public class GetSettingsResponse extends ActionResponse implements ToXContentObject { From 8e1eba22fcb871b2f5b34d5cfea142f59502977d Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Mon, 2 Apr 2018 12:13:51 -0400 Subject: [PATCH 03/11] Consolidate much of the get_settings logic inside of the tranpsort layer, moving logic from the rest layer Expand transport protocol to include defaults start with version 7 Add tests to ensure transport protocol backwards compatibility --- .../org/elasticsearch/client/Request.java | 6 +- .../elasticsearch/client/IndicesClientIT.java | 30 +-- .../elasticsearch/client/RequestTests.java | 18 +- .../elasticsearch/action/ActionModule.java | 2 +- .../settings/get/GetSettingsRequest.java | 32 ++++ .../settings/get/GetSettingsResponse.java | 151 +++++++++++++--- .../get/TransportGetSettingsAction.java | 24 ++- .../admin/indices/RestGetSettingsAction.java | 29 +-- .../get/GetSettingsResponseTests.java | 171 +++++++++++++++++- 9 files changed, 372 insertions(+), 91 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java index 85e760f7eef93..32b39b9f8030a 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java @@ -598,10 +598,10 @@ static Request getSettings(GetSettingsRequest getSettingsRequest) throws IOExcep Params params = Params.builder(); params.withIndicesOptions(getSettingsRequest.indicesOptions()); params.withLocal(getSettingsRequest.local()); + params.withFlatSettings(getSettingsRequest.flatSettings()); + params.withIncludeDefaults(getSettingsRequest.includeDefaults()); + params.withMasterTimeout(getSettingsRequest.masterNodeTimeout()); - if (getSettingsRequest.indices().length == 0) { - throw new IllegalArgumentException("getSettings requires at least one index"); - } String endpoint = endpoint(getSettingsRequest.indices(), "_settings", getSettingsRequest.names()); return new Request(HttpGet.METHOD_NAME, endpoint, params.getParams(), null); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java index ecb1d7f40fe19..d1fbb14cc68d2 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java @@ -194,20 +194,22 @@ public void testCreateIndex() throws IOException { public void testGetSettings() throws IOException { String indexName = "get_settings_index"; Settings basicSettings = Settings.builder() - .put("number_of_shards", 13) + .put("number_of_shards", 1) .put("number_of_replicas", 0) .build(); createIndex(indexName, basicSettings); GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indexName); - GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync); + GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, + highLevelClient().indices()::getSettingsAsync); assertNull(getSettingsResponse.getSetting(indexName, "index.refresh_interval")); - assertEquals("13", getSettingsResponse.getSetting(indexName, "index.number_of_shards")); + assertEquals("1", getSettingsResponse.getSetting(indexName, "index.number_of_shards")); updateIndexSettings(indexName, Settings.builder().put("refresh_interval", "30s")); - GetSettingsResponse updatedResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync); + GetSettingsResponse updatedResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, + highLevelClient().indices()::getSettingsAsync); assertEquals("30s", updatedResponse.getSetting(indexName, "index.refresh_interval")); } @@ -221,33 +223,35 @@ public void testGetSettingsNonExistentIndex() throws IOException { assertEquals(RestStatus.NOT_FOUND, exception.status()); } - public void testGetMultipleSettings() throws IOException { + public void testGetSettingsFromMultipleIndices() throws IOException { String indexName1 = "get_multiple_settings_one"; - createIndex(indexName1, Settings.builder().put("number_of_shards", 11).build()); + createIndex(indexName1, Settings.builder().put("number_of_shards", 2).build()); String indexName2 = "get_multiple_settings_two"; - createIndex(indexName2, Settings.builder().put("number_of_shards", 22).build()); + createIndex(indexName2, Settings.builder().put("number_of_shards", 3).build()); GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices("get_multiple_settings*"); - GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync); + GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, + highLevelClient().indices()::getSettingsAsync); - assertEquals("11", getSettingsResponse.getSetting(indexName1, "index.number_of_shards")); - assertEquals("22", getSettingsResponse.getSetting(indexName2, "index.number_of_shards")); + assertEquals("2", getSettingsResponse.getSetting(indexName1, "index.number_of_shards")); + assertEquals("3", getSettingsResponse.getSetting(indexName2, "index.number_of_shards")); } public void testGetSettingsFiltered() throws IOException { String indexName = "get_settings_index"; Settings basicSettings = Settings.builder() - .put("number_of_shards", 13) + .put("number_of_shards", 1) .put("number_of_replicas", 0) .build(); createIndex(indexName, basicSettings); GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indexName).names("index.number_of_shards"); - GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, highLevelClient().indices()::getSettingsAsync); + GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, + highLevelClient().indices()::getSettingsAsync); assertNull(getSettingsResponse.getSetting(indexName, "index.number_of_replicas")); - assertEquals("13", getSettingsResponse.getSetting(indexName, "index.number_of_shards")); + assertEquals("1", getSettingsResponse.getSetting(indexName, "index.number_of_shards")); assertEquals(1, getSettingsResponse.getIndexToSettings().get("get_settings_index").size()); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index 3e02639ac9e41..30798e612a006 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -430,22 +430,24 @@ public void testDeleteIndex() { } public void testGetSettings() throws IOException { - String[] indices = randomIndicesNames(1, 5); - GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indices); + String emptyIndexName = ""; + String nullIndexName = null; + String[] randomIndices = randomIndicesNames(1, 5); + String[] indicesUnderTest = java.util.stream.Stream.concat( + java.util.Arrays.stream(randomIndices), + java.util.stream.Stream.of(emptyIndexName, nullIndexName) + ).toArray(String[]::new); + + GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indicesUnderTest); Map expectedParams = new HashMap<>(); - /* HELP setRandomMasterTimeout(getSettingsRequest, expectedParams); - I definitely have some questions here. Enabling this causes the master timeout - parameter *not* to come through on the actual request, but I can't figure out - why. Further, I'm not entirely sure I need it. - */ setRandomIndicesOptions(getSettingsRequest::indicesOptions, getSettingsRequest::indicesOptions, expectedParams); setRandomLocal(getSettingsRequest, expectedParams); Request request = Request.getSettings(getSettingsRequest); - StringJoiner endpoint = new StringJoiner("/", "/", "").add(String.join(",", indices)).add("_settings"); + StringJoiner endpoint = new StringJoiner("/", "/", "").add(String.join(",", indicesUnderTest)).add("_settings"); assertThat(endpoint.toString(), equalTo(request.getEndpoint())); assertThat(request.getParameters(), equalTo(expectedParams)); assertThat(request.getMethod(), equalTo(HttpGet.METHOD_NAME)); diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 60ba0a43396e4..b21fecb618644 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -577,7 +577,7 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestOpenIndexAction(settings, restController)); registerHandler.accept(new RestUpdateSettingsAction(settings, restController)); - registerHandler.accept(new RestGetSettingsAction(settings, restController, indexScopedSettings, settingsFilter)); + registerHandler.accept(new RestGetSettingsAction(settings, restController)); registerHandler.accept(new RestAnalyzeAction(settings, restController)); registerHandler.accept(new RestGetIndexTemplateAction(settings, restController)); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java index 3a84543f34017..92b1a46d88f12 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java @@ -36,6 +36,8 @@ public class GetSettingsRequest extends MasterNodeReadRequest + * See + * Flat Settings flag on elastic.co + * This flag is specific to the rest client. + */ + public GetSettingsRequest flatSettings(boolean flatSettings) { + this.flatSettings = flatSettings; + return this; + } + + /** + * When include_defaults is set, return default values which are normally suppressed. + * This flag is specific to the rest client. + */ + public GetSettingsRequest includeDefaults(boolean includeDefaults) { + this.includeDefaults = includeDefaults; + return this; + } + + public GetSettingsRequest() { } @@ -96,6 +120,14 @@ public GetSettingsRequest humanReadable(boolean humanReadable) { return this; } + public boolean flatSettings() { + return flatSettings; + } + + public boolean includeDefaults() { + return includeDefaults; + } + @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java index c647adc758fa0..14601f02fd67f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java @@ -21,31 +21,40 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParserUtils; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.CharBuffer; import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import java.util.Objects; public class GetSettingsResponse extends ActionResponse implements ToXContentObject { private ImmutableOpenMap indexToSettings = ImmutableOpenMap.of(); + private ImmutableOpenMap indexToDefaultSettings = ImmutableOpenMap.of(); private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "get_index_settings_response", true, args -> new GetSettingsResponse() ); - public GetSettingsResponse(ImmutableOpenMap indexToSettings) { + public GetSettingsResponse(ImmutableOpenMap indexToSettings, + ImmutableOpenMap indexToDefaultSettings) { this.indexToSettings = indexToSettings; + this.indexToDefaultSettings = indexToDefaultSettings; } GetSettingsResponse() { @@ -57,8 +66,17 @@ public ImmutableOpenMap getIndexToSettings() { public String getSetting(String index, String setting) { Settings settings = indexToSettings.get(index); + Settings defaultSettings = indexToDefaultSettings.get(index); if (setting != null) { - return settings.get(setting); + if (settings.hasValue(setting)) { + return settings.get(setting); + } else { + if (defaultSettings != null) { + return defaultSettings.get(setting); + } else { + return null; + } + } } else { return null; } @@ -67,12 +85,22 @@ public String getSetting(String index, String setting) { @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); - int size = in.readVInt(); - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(); - for (int i = 0; i < size; i++) { - builder.put(in.readString(), Settings.readSettingsFromStream(in)); + + int settingsSize = in.readVInt(); + ImmutableOpenMap.Builder settingsBuilder = ImmutableOpenMap.builder(); + for (int i = 0; i < settingsSize; i++) { + settingsBuilder.put(in.readString(), Settings.readSettingsFromStream(in)); + } + ImmutableOpenMap.Builder defaultSettingsBuilder = ImmutableOpenMap.builder(); + + if (in.getVersion().onOrAfter(org.elasticsearch.Version.V_7_0_0_alpha1)) { + int defaultSettingsSize = in.readVInt(); + for (int i = 0; i < defaultSettingsSize ; i++) { + defaultSettingsBuilder.put(in.readString(), Settings.readSettingsFromStream(in)); + } } - indexToSettings = builder.build(); + indexToSettings = settingsBuilder.build(); + indexToDefaultSettings = defaultSettingsBuilder.build(); } @Override @@ -83,10 +111,52 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(cursor.key); Settings.writeSettingsToStream(cursor.value, out); } + if (out.getVersion().onOrAfter(org.elasticsearch.Version.V_7_0_0_alpha1)) { + out.writeVInt(indexToDefaultSettings.size()); + for (ObjectObjectCursor cursor : indexToDefaultSettings) { + out.writeString(cursor.key); + Settings.writeSettingsToStream(cursor.value, out); + } + } + } + + private static void validateSettingsFieldName(XContentParser parser) throws IOException { + String settingsFieldName = parser.currentName(); + XContentParserUtils.ensureExpectedToken(org.elasticsearch.common.xcontent.XContentParser.Token.FIELD_NAME, parser.currentToken(), + parser::getTokenLocation); + if (settingsFieldName.equals("settings") == false && settingsFieldName.equals("defaults") == false) { + String message = "Failed to parse object: expecting field with name [%s] but found [%s]"; + throw new org.elasticsearch.common.ParsingException(parser.getTokenLocation(), String.format(java.util.Locale.ROOT, message, + "(settings|defaults)", settingsFieldName)); + } + } + + private static void parseSettingsField(XContentParser parser, String currentIndexName, Map indexToSettings, + Map indexToDefaultSettings) throws IOException { + XContentParserUtils.ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.currentToken(), parser::getTokenLocation); + validateSettingsFieldName(parser); + String settingsFieldName = parser.currentName(); //this will be either "settings" or "defaults" + parser.nextToken(); //go to settings object itself + XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); + + switch (settingsFieldName) { + case "settings": + indexToSettings.put(currentIndexName, Settings.fromXContent(parser)); + break; + case "defaults": + indexToDefaultSettings.put(currentIndexName, Settings.fromXContent(parser)); + break; + default: + throw new IllegalStateException("Unknown field name for settings field! Should be \"settings\" or \"default\", but " + + "was: "+settingsFieldName); + + } } public static GetSettingsResponse fromXContent(XContentParser parser) throws IOException { HashMap indexToSettings = new HashMap<>(); + HashMap indexToDefaultSettings = new HashMap<>(); + if (parser.currentToken() == null) { parser.nextToken(); } @@ -97,35 +167,62 @@ public static GetSettingsResponse fromXContent(XContentParser parser) throws IOE String indexName = parser.currentName(); parser.nextToken(); //go to per-index settings object XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); - XContentParserUtils.ensureFieldName(parser, parser.nextToken(), "settings"); - parser.nextToken(); //go to settings object itself - XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); - Settings settings = Settings.fromXContent(parser); - parser.nextToken(); //end per-index object + parser.nextToken(); + + parseSettingsField(parser, indexName, indexToSettings, indexToDefaultSettings); //this will get one of settings or defaults + parser.nextToken(); + + if (parser.currentToken() == XContentParser.Token.FIELD_NAME) { + //we have both settings and default settings, let's grab the second one + parseSettingsField(parser, indexName, indexToSettings, indexToDefaultSettings); + parser.nextToken(); + } + XContentParserUtils.ensureExpectedToken(XContentParser.Token.END_OBJECT, parser.currentToken(), parser::getTokenLocation); parser.nextToken(); //we should now be positioned at the beginning of the next index's settings, or at the end - indexToSettings.put(indexName, settings); } - return new GetSettingsResponse(ImmutableOpenMap.builder().putAll(indexToSettings).build()); + + ImmutableOpenMap settingsMap = ImmutableOpenMap.builder().putAll(indexToSettings).build(); + ImmutableOpenMap defaultSettingsMap = + ImmutableOpenMap.builder().putAll(indexToDefaultSettings).build(); + + + return new GetSettingsResponse(settingsMap, defaultSettingsMap); } + @Override public String toString() { - return indexToSettings.toString(); + try { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + XContentBuilder builder = new XContentBuilder(JsonXContent.jsonXContent, baos); + toXContent(builder, ToXContent.EMPTY_PARAMS, false); + return Strings.toString(builder); + } catch (IOException e) { + throw new IllegalStateException(e); //should not be possible here + } } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return toXContent(builder, params, true); + } + + private XContentBuilder toXContent(XContentBuilder builder, Params params, boolean omitEmptySettings) throws IOException { builder.startObject(); - Iterator indicesIterator = indexToSettings.keysIt(); - while (indicesIterator.hasNext()) { - String indexName = indicesIterator.next(); - Settings settings = indexToSettings.get(indexName); - builder.field(indexName); - builder.startObject(); - builder.field("settings"); - builder.startObject(); - settings.toXContent(builder, params); + for (ObjectObjectCursor cursor : getIndexToSettings()) { + // no settings, jump over it to shorten the response data + if (omitEmptySettings && cursor.value.isEmpty()) { + continue; + } + builder.startObject(cursor.key); + builder.startObject("settings"); + cursor.value.toXContent(builder, params); builder.endObject(); + if (!indexToDefaultSettings.isEmpty()) { + builder.startObject("defaults"); + indexToDefaultSettings.get(cursor.key).toXContent(builder, params); + builder.endObject(); + } builder.endObject(); } builder.endObject(); @@ -137,12 +234,12 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; GetSettingsResponse that = (GetSettingsResponse) o; - return Objects.equals(indexToSettings, that.indexToSettings); + return Objects.equals(indexToSettings, that.indexToSettings) && + Objects.equals(indexToDefaultSettings, that.indexToDefaultSettings); } @Override public int hashCode() { - - return Objects.hash(indexToSettings); + return Objects.hash(indexToSettings, indexToDefaultSettings); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java index 3109fa4d405ac..3cc66822ff9ca 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java @@ -37,19 +37,23 @@ import org.elasticsearch.index.Index; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.common.settings.IndexScopedSettings; -import java.util.Map; +import java.util.Arrays; public class TransportGetSettingsAction extends TransportMasterNodeReadAction { private final SettingsFilter settingsFilter; + private final IndexScopedSettings indexScopedSettings; + @Inject public TransportGetSettingsAction(Settings settings, TransportService transportService, ClusterService clusterService, ThreadPool threadPool, SettingsFilter settingsFilter, ActionFilters actionFilters, - IndexNameExpressionResolver indexNameExpressionResolver) { + IndexNameExpressionResolver indexNameExpressionResolver, IndexScopedSettings indexedScopedSettings) { super(settings, GetSettingsAction.NAME, transportService, clusterService, threadPool, actionFilters, GetSettingsRequest::new, indexNameExpressionResolver); this.settingsFilter = settingsFilter; + this.indexScopedSettings = indexedScopedSettings; } @Override @@ -73,21 +77,27 @@ protected GetSettingsResponse newResponse() { protected void masterOperation(GetSettingsRequest request, ClusterState state, ActionListener listener) { Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, request); ImmutableOpenMap.Builder indexToSettingsBuilder = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder indexToDefaultSettingsBuilder = ImmutableOpenMap.builder(); for (Index concreteIndex : concreteIndices) { IndexMetaData indexMetaData = state.getMetaData().index(concreteIndex); if (indexMetaData == null) { continue; } - Settings settings = settingsFilter.filter(indexMetaData.getSettings()); + Settings indexSettings = settingsFilter.filter(indexMetaData.getSettings()); if (request.humanReadable()) { - settings = IndexMetaData.addHumanReadableSettings(settings); + indexSettings = IndexMetaData.addHumanReadableSettings(this.settings); } + if (CollectionUtils.isEmpty(request.names()) == false) { - settings = settings.filter(k -> Regex.simpleMatch(request.names(), k)); + indexSettings = indexSettings.filter(k -> Regex.simpleMatch(request.names(), k)); + } + + indexToSettingsBuilder.put(concreteIndex.getName(), indexSettings); + if (request.includeDefaults()) { + indexToDefaultSettingsBuilder.put(concreteIndex.getName(), indexScopedSettings.diff(indexSettings, this.settings)); } - indexToSettingsBuilder.put(concreteIndex.getName(), settings); } - listener.onResponse(new GetSettingsResponse(indexToSettingsBuilder.build())); + listener.onResponse(new GetSettingsResponse(indexToSettingsBuilder.build(), indexToDefaultSettingsBuilder.build())); } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetSettingsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetSettingsAction.java index 8ac7f12312a45..9791994c773e2 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetSettingsAction.java @@ -44,18 +44,12 @@ public class RestGetSettingsAction extends BaseRestHandler { - private final IndexScopedSettings indexScopedSettings; - private final SettingsFilter settingsFilter; - - public RestGetSettingsAction(Settings settings, RestController controller, IndexScopedSettings indexScopedSettings, - final SettingsFilter settingsFilter) { + public RestGetSettingsAction(Settings settings, RestController controller) { super(settings); - this.indexScopedSettings = indexScopedSettings; controller.registerHandler(GET, "/_settings/{name}", this); controller.registerHandler(GET, "/{index}/_settings", this); controller.registerHandler(GET, "/{index}/_settings/{name}", this); controller.registerHandler(GET, "/{index}/_setting/{name}", this); - this.settingsFilter = settingsFilter; } @Override @@ -73,31 +67,16 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC .indices(Strings.splitStringByCommaToArray(request.param("index"))) .indicesOptions(IndicesOptions.fromRequest(request, IndicesOptions.strictExpandOpen())) .humanReadable(request.hasParam("human")) + .includeDefaults(renderDefaults) .names(names); getSettingsRequest.local(request.paramAsBoolean("local", getSettingsRequest.local())); + getSettingsRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getSettingsRequest.masterNodeTimeout())); return channel -> client.admin().indices().getSettings(getSettingsRequest, new RestBuilderListener(channel) { @Override public RestResponse buildResponse(GetSettingsResponse getSettingsResponse, XContentBuilder builder) throws Exception { - builder.startObject(); - for (ObjectObjectCursor cursor : getSettingsResponse.getIndexToSettings()) { - // no settings, jump over it to shorten the response data - if (cursor.value.isEmpty()) { - continue; - } - builder.startObject(cursor.key); - builder.startObject("settings"); - cursor.value.toXContent(builder, request); - builder.endObject(); - if (renderDefaults) { - builder.startObject("defaults"); - settingsFilter.filter(indexScopedSettings.diff(cursor.value, settings)).toXContent(builder, request); - builder.endObject(); - } - builder.endObject(); - } - builder.endObject(); + getSettingsResponse.toXContent(builder, request); return new BytesRestResponse(OK, builder); } }); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java index 173f21d29d084..c28e9662d1a0c 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java @@ -1,15 +1,71 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.elasticsearch.action.admin.indices.settings.get; +import org.elasticsearch.Version; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.RandomCreateIndexGenerator; import org.elasticsearch.test.AbstractStreamableXContentTestCase; +import org.junit.Assert; +import org.junit.Test; import java.io.IOException; +import java.util.Base64; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.Set; +import java.util.function.Predicate; + +import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; +import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; +import static org.elasticsearch.index.IndexSettings.INDEX_REFRESH_INTERVAL_SETTING; public class GetSettingsResponseTests extends AbstractStreamableXContentTestCase { + + private final Set indexNames; + public GetSettingsResponseTests() { + Set names = new HashSet(); + int numIndices = randomIntBetween(1, 5); + for (int x=0;x indexToSettings = new HashMap<>(); - int numIndices = randomIntBetween(1, 5); - for (int x=0;x indexToDefaultSettings = new HashMap<>(); + + IndexScopedSettings indexScopedSettings = IndexScopedSettings.DEFAULT_SCOPED_SETTINGS; + + for (String indexName : indexNames) { + Settings.Builder builder = Settings.builder(); + builder.put(RandomCreateIndexGenerator.randomIndexSettings()); + /* + We must ensure that *something* is in the settings response as we optimize away empty settings + blocks in x content responses + */ + builder.put("index.refresh_interval", "1s"); + indexToSettings.put(indexName, builder.build()); } - ImmutableOpenMap immutableIndexToSettings = ImmutableOpenMap.builder().putAll(indexToSettings).build(); - return new GetSettingsResponse(immutableIndexToSettings); + ImmutableOpenMap immutableIndexToSettings = + ImmutableOpenMap.builder().putAll(indexToSettings).build(); + + + if (randomBoolean()) { + for (String indexName : indexToSettings.keySet()) { + Settings defaultSettings = indexScopedSettings.diff(indexToSettings.get(indexName), Settings.EMPTY); + indexToDefaultSettings.put(indexName, defaultSettings); + } + } + + ImmutableOpenMap immutableIndexToDefaultSettings = + ImmutableOpenMap.builder().putAll(indexToDefaultSettings).build(); + + return new GetSettingsResponse(immutableIndexToSettings, immutableIndexToDefaultSettings); } @Override @@ -32,8 +110,87 @@ protected GetSettingsResponse doParseInstance(XContentParser parser) throws IOEx return GetSettingsResponse.fromXContent(parser); } - @Override protected boolean supportsUnknownFields() { return false; } + + private static final GetSettingsResponse getExpectedTest622Response() { + /* This is a fairly direct copy of the code used to generate the base64'd response above -- with the caveat that the constructor + has been modified so that the code compiles on this version of elasticsearch + */ + HashMap indexToSettings = new HashMap<>(); + Settings.Builder builder = Settings.builder(); + + builder.put(SETTING_NUMBER_OF_SHARDS, 2); + builder.put(SETTING_NUMBER_OF_REPLICAS, 1); + indexToSettings.put("index_name", builder.build()); + GetSettingsResponse response = new GetSettingsResponse(ImmutableOpenMap.builder().putAll(indexToSettings).build + (), ImmutableOpenMap.of()); + return response; + } + + private static final GetSettingsResponse getResponseWithNewFields() { + HashMap indexToDefaultSettings = new HashMap<>(); + Settings.Builder builder = Settings.builder(); + + builder.put(INDEX_REFRESH_INTERVAL_SETTING.getKey(), "1s"); + indexToDefaultSettings.put("index_name", builder.build()); + ImmutableOpenMap defaultsMap = ImmutableOpenMap.builder().putAll(indexToDefaultSettings) + .build(); + return new GetSettingsResponse(getExpectedTest622Response().getIndexToSettings(), defaultsMap); + } + + @Test + public void testCanDecode622Response() throws IOException { + StreamInput si = StreamInput.wrap(Base64.getDecoder().decode(TEST_6_2_2_RESPONSE_BYTES)); + si.setVersion(Version.V_6_2_2); + GetSettingsResponse response = new GetSettingsResponse(); + response.readFrom(si); + + Assert.assertEquals(TEST_6_2_2_RESPONSE_INSTANCE, response); + } + + @Test + public void testCanOutput622Response() throws IOException { + GetSettingsResponse responseWithExtraFields = getResponseWithNewFields(); + BytesStreamOutput bso = new BytesStreamOutput(); + bso.setVersion(Version.V_6_2_2); + responseWithExtraFields.writeTo(bso); + + String base64OfResponse = Base64.getEncoder().encodeToString(BytesReference.toBytes(bso.bytes())); + + Assert.assertEquals(TEST_6_2_2_RESPONSE_BYTES, base64OfResponse); + } + + @Test + public void testTransportSerdeRoundTripCurrentVersion() throws IOException { + GetSettingsResponse responseWithExtraFields = getResponseWithNewFields(); + BytesStreamOutput bso = new BytesStreamOutput(); + responseWithExtraFields.writeTo(bso); + + byte[] responseBytes = BytesReference.toBytes(bso.bytes()); + StreamInput si = StreamInput.wrap(responseBytes); + + GetSettingsResponse newResponse = new GetSettingsResponse(); + newResponse.readFrom(si); + + Assert.assertEquals(responseWithExtraFields, newResponse); + } + @Test + public void testRoundtrip622DropsDefaults() throws IOException { + GetSettingsResponse responseWithExtraFields = getResponseWithNewFields(); + BytesStreamOutput bso = new BytesStreamOutput(); + bso.setVersion(Version.V_6_2_2); + responseWithExtraFields.writeTo(bso); + + byte[] responseBytes = BytesReference.toBytes(bso.bytes()); + StreamInput si = StreamInput.wrap(responseBytes); + si.setVersion(Version.V_6_2_2); + + GetSettingsResponse newResponse = new GetSettingsResponse(); + newResponse.readFrom(si); + + Assert.assertNotEquals(responseWithExtraFields, newResponse); + Assert.assertEquals(getExpectedTest622Response(), newResponse); + } } From a7e87c541c2c16a8fc44f29b8c2acc3fe5f8d861 Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Mon, 2 Apr 2018 13:17:49 -0400 Subject: [PATCH 04/11] fix a few minor nits --- .../settings/get/GetSettingsResponseTests.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java index c28e9662d1a0c..0e58a8724b197 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java @@ -30,7 +30,6 @@ import org.elasticsearch.index.RandomCreateIndexGenerator; import org.elasticsearch.test.AbstractStreamableXContentTestCase; import org.junit.Assert; -import org.junit.Test; import java.io.IOException; import java.util.Base64; @@ -38,7 +37,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Set; -import java.util.function.Predicate; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; @@ -114,7 +112,7 @@ protected boolean supportsUnknownFields() { return false; } - private static final GetSettingsResponse getExpectedTest622Response() { + private static GetSettingsResponse getExpectedTest622Response() { /* This is a fairly direct copy of the code used to generate the base64'd response above -- with the caveat that the constructor has been modified so that the code compiles on this version of elasticsearch */ @@ -129,7 +127,7 @@ private static final GetSettingsResponse getExpectedTest622Response() { return response; } - private static final GetSettingsResponse getResponseWithNewFields() { + private static GetSettingsResponse getResponseWithNewFields() { HashMap indexToDefaultSettings = new HashMap<>(); Settings.Builder builder = Settings.builder(); @@ -140,7 +138,6 @@ private static final GetSettingsResponse getResponseWithNewFields() { return new GetSettingsResponse(getExpectedTest622Response().getIndexToSettings(), defaultsMap); } - @Test public void testCanDecode622Response() throws IOException { StreamInput si = StreamInput.wrap(Base64.getDecoder().decode(TEST_6_2_2_RESPONSE_BYTES)); si.setVersion(Version.V_6_2_2); @@ -150,7 +147,6 @@ public void testCanDecode622Response() throws IOException { Assert.assertEquals(TEST_6_2_2_RESPONSE_INSTANCE, response); } - @Test public void testCanOutput622Response() throws IOException { GetSettingsResponse responseWithExtraFields = getResponseWithNewFields(); BytesStreamOutput bso = new BytesStreamOutput(); @@ -162,7 +158,6 @@ public void testCanOutput622Response() throws IOException { Assert.assertEquals(TEST_6_2_2_RESPONSE_BYTES, base64OfResponse); } - @Test public void testTransportSerdeRoundTripCurrentVersion() throws IOException { GetSettingsResponse responseWithExtraFields = getResponseWithNewFields(); BytesStreamOutput bso = new BytesStreamOutput(); @@ -176,7 +171,7 @@ public void testTransportSerdeRoundTripCurrentVersion() throws IOException { Assert.assertEquals(responseWithExtraFields, newResponse); } - @Test + public void testRoundtrip622DropsDefaults() throws IOException { GetSettingsResponse responseWithExtraFields = getResponseWithNewFields(); BytesStreamOutput bso = new BytesStreamOutput(); From 745813dd4aa714df4fb9a109f10e413069c6d30f Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Fri, 6 Apr 2018 14:54:29 -0400 Subject: [PATCH 05/11] incorporate PR feedback --- .../elasticsearch/client/RequestTests.java | 8 +-- .../IndicesClientDocumentationIT.java | 59 ++++++++++++++++--- .../high-level/indices/get_settings.asciidoc | 11 +++- .../get/TransportGetSettingsAction.java | 11 ++-- .../get/TransportGetSettingsActionIT.java | 21 +++++++ 5 files changed, 88 insertions(+), 22 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index 30798e612a006..4e2777b205abd 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -430,13 +430,7 @@ public void testDeleteIndex() { } public void testGetSettings() throws IOException { - String emptyIndexName = ""; - String nullIndexName = null; - String[] randomIndices = randomIndicesNames(1, 5); - String[] indicesUnderTest = java.util.stream.Stream.concat( - java.util.Arrays.stream(randomIndices), - java.util.stream.Stream.of(emptyIndexName, nullIndexName) - ).toArray(String[]::new); + String[] indicesUnderTest = randomBoolean() ? null : randomIndicesNames(0, 5); GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indicesUnderTest); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java index 8a0989d27ea39..3d2b8c893b6a4 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java @@ -795,16 +795,14 @@ public void testGetSettings() throws Exception { request.names("index.number_of_shards"); // <1> // end::get-settings-request-names - // tag::get-settings-request-masterTimeout - request.masterNodeTimeout(TimeValue.timeValueMinutes(1)); // <1> - request.masterNodeTimeout("1m"); // <2> - // end::get-settings-request-masterTimeout + // tag::get-settings-flat-settings + request.flatSettings(); // <1> + // end::get-settings-flat-settings // tag::get-settings-request-indicesOptions request.indicesOptions(IndicesOptions.lenientExpandOpen()); // <1> // end::get-settings-request-indicesOptions - // tag::get-settings-execute GetSettingsResponse getSettingsResponse = client.indices().getSettings(request); // end::get-settings-execute @@ -812,11 +810,13 @@ public void testGetSettings() throws Exception { // tag::get-settings-response String numberOfShardsString = getSettingsResponse.getSetting("index", "index.number_of_shards"); // <1> Settings indexSettings = getSettingsResponse.getIndexToSettings().get("index"); // <2> - int numberOfShards = indexSettings.getAsInt("index.number_of_shards", null); // <3> + Integer numberOfShards = indexSettings.getAsInt("index.number_of_shards", null); // <3> // end::get-settings-response assertEquals("3", numberOfShardsString); - assertEquals(3, numberOfShards); + assertEquals(Integer.valueOf(3), numberOfShards); + + assertNull("refresh_interval returned but was never set!", indexSettings.get("index.refresh_interval")); // tag::get-settings-execute-listener ActionListener listener = @@ -842,10 +842,55 @@ public void onFailure(Exception e) { // end::get-settings-execute-async assertTrue(latch.await(30L, TimeUnit.SECONDS)); + } + public void testGetSettingsWithDefaults() throws Exception { + RestHighLevelClient client = highLevelClient(); + { + Settings settings = Settings.builder().put("number_of_shards", 3).build(); + CreateIndexResponse createIndexResponse = client.indices().create(new CreateIndexRequest("index", settings)); + assertTrue(createIndexResponse.isAcknowledged()); + } + GetSettingsRequest request = new GetSettingsRequest().indices("index"); + request.names("index.number_of_shards"); // <1> + request.flatSettings(); // <1> + request.indicesOptions(IndicesOptions.lenientExpandOpen()); // <1> + + // tag::get-settings-include-defaults + request.includeDefaults(); // <1> + // end::get-settings-include-defaults + + GetSettingsResponse getSettingsResponse = client.indices().getSettings(request); + String numberOfShardsString = getSettingsResponse.getSetting("index", "index.number_of_shards"); // <1> + Settings indexSettings = getSettingsResponse.getIndexToSettings().get("index"); // <2> + Integer numberOfShards = indexSettings.getAsInt("index.number_of_shards", null); // <3> + + assertEquals("3", numberOfShardsString); + assertEquals(Integer.valueOf(3), numberOfShards); + assertNotNull("with defaults enabled we should get a value for refresh_interval!", indexSettings.get("index.refresh_interval")); + + ActionListener listener = + new ActionListener() { + @Override + public void onResponse(GetSettingsResponse GetSettingsResponse) { + // <1> + } + + @Override + public void onFailure(Exception e) { + // <2> + } + }; + + // Replace the empty listener by a blocking listener in test + final CountDownLatch latch = new CountDownLatch(1); + listener = new LatchedActionListener<>(listener, latch); + + client.indices().getSettingsAsync(request, listener); // <1> + assertTrue(latch.await(30L, TimeUnit.SECONDS)); } public void testForceMergeIndex() throws Exception { diff --git a/docs/java-rest/high-level/indices/get_settings.asciidoc b/docs/java-rest/high-level/indices/get_settings.asciidoc index c4bf1abb61b83..0b599463f1cce 100644 --- a/docs/java-rest/high-level/indices/get_settings.asciidoc +++ b/docs/java-rest/high-level/indices/get_settings.asciidoc @@ -23,10 +23,15 @@ include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-reque ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request-masterTimeout] +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request-flat-settings] -------------------------------------------------- -<1> Timeout to connect to the master node as a `TimeValue` -<2> Timeout to connect to the master node as a `String` +<1> If set, the HTTP response body will represent settings as flat lists rather than nested objects + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request-include-defaults] +-------------------------------------------------- +<1> If set, defaults will be returned for settings not explicitly set on the index ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java index 3cc66822ff9ca..ab2bdbb22fe13 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java @@ -84,18 +84,19 @@ protected void masterOperation(GetSettingsRequest request, ClusterState state, A continue; } - Settings indexSettings = settingsFilter.filter(indexMetaData.getSettings()); + Settings settings = settingsFilter.filter(indexMetaData.getSettings()); if (request.humanReadable()) { - indexSettings = IndexMetaData.addHumanReadableSettings(this.settings); + settings = IndexMetaData.addHumanReadableSettings(settings); } if (CollectionUtils.isEmpty(request.names()) == false) { - indexSettings = indexSettings.filter(k -> Regex.simpleMatch(request.names(), k)); + settings = settings.filter(k -> Regex.simpleMatch(request.names(), k)); } - indexToSettingsBuilder.put(concreteIndex.getName(), indexSettings); + indexToSettingsBuilder.put(concreteIndex.getName(), settings); if (request.includeDefaults()) { - indexToDefaultSettingsBuilder.put(concreteIndex.getName(), indexScopedSettings.diff(indexSettings, this.settings)); + Settings defaultSettings = settingsFilter.filter(indexScopedSettings.diff(settings, this.settings)); + indexToDefaultSettingsBuilder.put(concreteIndex.getName(), defaultSettings); } } listener.onResponse(new GetSettingsResponse(indexToSettingsBuilder.build(), indexToDefaultSettingsBuilder.build())); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java new file mode 100644 index 0000000000000..e44eec7f812ee --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java @@ -0,0 +1,21 @@ +package org.elasticsearch.action.admin.indices.settings.get; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESIntegTestCase; + +public class TransportGetSettingsActionIT extends ESIntegTestCase { + public void testIncludeDefaults() { + String indexName = "test_index"; + createIndex(indexName, Settings.EMPTY); + GetSettingsRequest noDefaultsRequest = new GetSettingsRequest().indices(indexName); + GetSettingsResponse noDefaultsResponse = client().admin().indices().getSettings(noDefaultsRequest).actionGet(); + assertNull("index.refresh_interval should be null as it was never set", noDefaultsResponse.getSetting(indexName, + "index.refresh_interval")); + + GetSettingsRequest defaultsRequest = new GetSettingsRequest().indices(indexName).includeDefaults(true); + GetSettingsResponse defaultsResponse = client().admin().indices().getSettings(defaultsRequest).actionGet(); + assertNotNull("index.refresh_interval should be set as we are including defaults", defaultsResponse.getSetting(indexName, + "index.refresh_interval")); + + } +} From 906b32af17891a5c924ae1f1afa0598034cdd079 Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Wed, 11 Apr 2018 14:16:42 -0400 Subject: [PATCH 06/11] address reviewer feedback --- .../org/elasticsearch/client/Request.java | 4 +- .../elasticsearch/client/IndicesClientIT.java | 38 +++++++++++ .../elasticsearch/client/RequestTests.java | 8 ++- .../IndicesClientDocumentationIT.java | 32 +++++---- .../high-level/indices/get_settings.asciidoc | 18 +++-- .../settings/get/GetSettingsRequest.java | 47 ++++++++----- .../settings/get/GetSettingsResponse.java | 39 +++++------ .../get/TransportGetSettingsAction.java | 19 ++++-- .../settings/get/GetSettingsRequestTests.java | 67 +++++++++++++++++++ .../get/TransportGetSettingsActionIT.java | 33 ++++++++- 10 files changed, 230 insertions(+), 75 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequestTests.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java index 32b39b9f8030a..7eddbf51d313a 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java @@ -595,14 +595,14 @@ static Request rollover(RolloverRequest rolloverRequest) throws IOException { } static Request getSettings(GetSettingsRequest getSettingsRequest) throws IOException { + String[] indices = getSettingsRequest.indices() == null ? Strings.EMPTY_ARRAY : getSettingsRequest.indices(); Params params = Params.builder(); params.withIndicesOptions(getSettingsRequest.indicesOptions()); params.withLocal(getSettingsRequest.local()); - params.withFlatSettings(getSettingsRequest.flatSettings()); params.withIncludeDefaults(getSettingsRequest.includeDefaults()); params.withMasterTimeout(getSettingsRequest.masterNodeTimeout()); - String endpoint = endpoint(getSettingsRequest.indices(), "_settings", getSettingsRequest.names()); + String endpoint = endpoint(indices, "_settings", getSettingsRequest.names()); return new Request(HttpGet.METHOD_NAME, endpoint, params.getParams(), null); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java index d1fbb14cc68d2..eb09084200bd2 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java @@ -255,6 +255,44 @@ public void testGetSettingsFiltered() throws IOException { assertEquals(1, getSettingsResponse.getIndexToSettings().get("get_settings_index").size()); } + public void testGetSettingsWithDefaults() throws IOException { + String indexName = "get_settings_index"; + Settings basicSettings = Settings.builder() + .put("number_of_shards", 1) + .put("number_of_replicas", 0) + .build(); + createIndex(indexName, basicSettings); + + GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indexName).includeDefaults(true); + GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, + highLevelClient().indices()::getSettingsAsync); + + assertNotNull(getSettingsResponse.getSetting(indexName, "index.refresh_interval")); + assertEquals(IndexSettings.DEFAULT_REFRESH_INTERVAL, + getSettingsResponse.getIndexToDefaultSettings().get("get_settings_index").getAsTime("index.refresh_interval", null)); + assertEquals("1", getSettingsResponse.getSetting(indexName, "index.number_of_shards")); + } + + public void testGetSettingsWithDefaultsFiltered() throws IOException { + String indexName = "get_settings_index"; + Settings basicSettings = Settings.builder() + .put("number_of_shards", 1) + .put("number_of_replicas", 0) + .build(); + createIndex(indexName, basicSettings); + + GetSettingsRequest getSettingsRequest = new GetSettingsRequest() + .indices(indexName) + .names("index.refresh_interval") + .includeDefaults(true); + GetSettingsResponse getSettingsResponse = execute(getSettingsRequest, highLevelClient().indices()::getSettings, + highLevelClient().indices()::getSettingsAsync); + + assertNull(getSettingsResponse.getSetting(indexName, "index.number_of_replicas")); + assertNull(getSettingsResponse.getSetting(indexName, "index.number_of_shards")); + assertEquals(0, getSettingsResponse.getIndexToSettings().get("get_settings_index").size()); + assertEquals(1, getSettingsResponse.getIndexToDefaultSettings().get("get_settings_index").size()); + } public void testPutMapping() throws IOException { { // Add mappings to index diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index 4e2777b205abd..848b0e8d4d217 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -441,7 +441,11 @@ public void testGetSettings() throws IOException { setRandomLocal(getSettingsRequest, expectedParams); Request request = Request.getSettings(getSettingsRequest); - StringJoiner endpoint = new StringJoiner("/", "/", "").add(String.join(",", indicesUnderTest)).add("_settings"); + StringJoiner endpoint = new StringJoiner("/", "/", ""); + if (indicesUnderTest != null && indicesUnderTest.length > 0) { + endpoint.add(String.join(",", indicesUnderTest)); + } + endpoint.add("_settings"); assertThat(endpoint.toString(), equalTo(request.getEndpoint())); assertThat(request.getParameters(), equalTo(expectedParams)); assertThat(request.getMethod(), equalTo(HttpGet.METHOD_NAME)); @@ -576,7 +580,7 @@ public void testIndex() throws IOException { } public void testRefresh() { - String[] indices = randomBoolean() ? null : randomIndicesNames(0, 5); + String[] indices = randomIndicesNames(1, 5); RefreshRequest refreshRequest; if (randomBoolean()) { refreshRequest = new RefreshRequest(indices); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java index 3d2b8c893b6a4..991004f1da876 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java @@ -795,10 +795,6 @@ public void testGetSettings() throws Exception { request.names("index.number_of_shards"); // <1> // end::get-settings-request-names - // tag::get-settings-flat-settings - request.flatSettings(); // <1> - // end::get-settings-flat-settings - // tag::get-settings-request-indicesOptions request.indicesOptions(IndicesOptions.lenientExpandOpen()); // <1> // end::get-settings-request-indicesOptions @@ -816,7 +812,8 @@ public void testGetSettings() throws Exception { assertEquals("3", numberOfShardsString); assertEquals(Integer.valueOf(3), numberOfShards); - assertNull("refresh_interval returned but was never set!", indexSettings.get("index.refresh_interval")); + assertNull("refresh_interval returned but was never set!", + getSettingsResponse.getSetting("index", "index.refresh_interval")); // tag::get-settings-execute-listener ActionListener listener = @@ -854,34 +851,35 @@ public void testGetSettingsWithDefaults() throws Exception { } GetSettingsRequest request = new GetSettingsRequest().indices("index"); - request.names("index.number_of_shards"); // <1> - request.flatSettings(); // <1> - request.indicesOptions(IndicesOptions.lenientExpandOpen()); // <1> + request.indicesOptions(IndicesOptions.lenientExpandOpen()); // tag::get-settings-include-defaults - request.includeDefaults(); // <1> + request.includeDefaults(true); // <1> // end::get-settings-include-defaults GetSettingsResponse getSettingsResponse = client.indices().getSettings(request); - String numberOfShardsString = getSettingsResponse.getSetting("index", "index.number_of_shards"); // <1> - Settings indexSettings = getSettingsResponse.getIndexToSettings().get("index"); // <2> - Integer numberOfShards = indexSettings.getAsInt("index.number_of_shards", null); // <3> + String numberOfShardsString = getSettingsResponse.getSetting("index", "index.number_of_shards"); + Settings indexSettings = getSettingsResponse.getIndexToSettings().get("index"); + Integer numberOfShards = indexSettings.getAsInt("index.number_of_shards", null); + + // tag::get-settings-defaults-response + String refreshInterval = getSettingsResponse.getSetting("index", "index.refresh_interval"); // <1> + Settings indexDefaultSettings = getSettingsResponse.getIndexToDefaultSettings().get("index"); // <2> + // end::get-settings-defaults-response assertEquals("3", numberOfShardsString); assertEquals(Integer.valueOf(3), numberOfShards); + assertNotNull("with defaults enabled we should get a value for refresh_interval!", refreshInterval); - assertNotNull("with defaults enabled we should get a value for refresh_interval!", indexSettings.get("index.refresh_interval")); - + assertEquals(refreshInterval, indexDefaultSettings.get("index.refresh_interval")); ActionListener listener = new ActionListener() { @Override public void onResponse(GetSettingsResponse GetSettingsResponse) { - // <1> } @Override public void onFailure(Exception e) { - // <2> } }; @@ -889,7 +887,7 @@ public void onFailure(Exception e) { final CountDownLatch latch = new CountDownLatch(1); listener = new LatchedActionListener<>(listener, latch); - client.indices().getSettingsAsync(request, listener); // <1> + client.indices().getSettingsAsync(request, listener); assertTrue(latch.await(30L, TimeUnit.SECONDS)); } diff --git a/docs/java-rest/high-level/indices/get_settings.asciidoc b/docs/java-rest/high-level/indices/get_settings.asciidoc index 0b599463f1cce..b054715119ec3 100644 --- a/docs/java-rest/high-level/indices/get_settings.asciidoc +++ b/docs/java-rest/high-level/indices/get_settings.asciidoc @@ -21,17 +21,11 @@ include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-reque -------------------------------------------------- <1> One or more settings that be the only settings retrieved. If unset, all settings will be retrieved -["source","java",subs="attributes,callouts,macros"] --------------------------------------------------- -include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request-flat-settings] --------------------------------------------------- -<1> If set, the HTTP response body will represent settings as flat lists rather than nested objects - ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-request-include-defaults] -------------------------------------------------- -<1> If set, defaults will be returned for settings not explicitly set on the index +<1> If true, defaults will be returned for settings not explicitly set on the index ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- @@ -90,3 +84,13 @@ include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-respo <1> We can retrieve the setting value for a particular index directly from the response as a string <2> We can also retrieve the Settings object for a particular index for further examination <3> The returned Settings object provides convenience methods for non String types + +If the `includeDefaults` flag was set to true in the `GetSettingsRequest`, the +behavior of `GetSettingsResponse` will differ somewhat. + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-settings-defaults-response] +-------------------------------------------------- +<1> Individual default setting values may be retrieved directly from the `GetSettingsResponse` +<2> We may retrieve a Settings object for an index that contains those settings with default values diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java index 92b1a46d88f12..55c55ce72d886 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.indices.settings.get; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.ValidateActions; @@ -29,6 +30,8 @@ import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; +import java.util.Arrays; +import java.util.Objects; public class GetSettingsRequest extends MasterNodeReadRequest implements IndicesRequest.Replaceable { @@ -37,7 +40,6 @@ public class GetSettingsRequest extends MasterNodeReadRequest - * See - * Flat Settings flag on elastic.co - * This flag is specific to the rest client. - */ - public GetSettingsRequest flatSettings(boolean flatSettings) { - this.flatSettings = flatSettings; - return this; - } - /** * When include_defaults is set, return default values which are normally suppressed. * This flag is specific to the rest client. @@ -81,6 +71,9 @@ public GetSettingsRequest(StreamInput in) throws IOException { indicesOptions = IndicesOptions.readIndicesOptions(in); names = in.readStringArray(); humanReadable = in.readBoolean(); + if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + includeDefaults = in.readBoolean(); + } } @Override @@ -90,6 +83,9 @@ public void writeTo(StreamOutput out) throws IOException { indicesOptions.writeIndicesOptions(out); out.writeStringArray(names); out.writeBoolean(humanReadable); + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + out.writeBoolean(includeDefaults); + } } @Override @@ -120,10 +116,6 @@ public GetSettingsRequest humanReadable(boolean humanReadable) { return this; } - public boolean flatSettings() { - return flatSettings; - } - public boolean includeDefaults() { return includeDefaults; } @@ -141,4 +133,25 @@ public ActionRequestValidationException validate() { public void readFrom(StreamInput in) throws IOException { throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + GetSettingsRequest that = (GetSettingsRequest) o; + return humanReadable == that.humanReadable && + includeDefaults == that.includeDefaults && + Arrays.equals(indices, that.indices) && + Objects.equals(indicesOptions, that.indicesOptions) && + Arrays.equals(names, that.names); + } + + @Override + public int hashCode() { + + int result = Objects.hash(indicesOptions, humanReadable, includeDefaults); + result = 31 * result + Arrays.hashCode(indices); + result = 31 * result + Arrays.hashCode(names); + return result; + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java index 14601f02fd67f..5500129ee4fb6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java @@ -47,10 +47,6 @@ public class GetSettingsResponse extends ActionResponse implements ToXContentObj private ImmutableOpenMap indexToSettings = ImmutableOpenMap.of(); private ImmutableOpenMap indexToDefaultSettings = ImmutableOpenMap.of(); - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - "get_index_settings_response", true, args -> new GetSettingsResponse() - ); - public GetSettingsResponse(ImmutableOpenMap indexToSettings, ImmutableOpenMap indexToDefaultSettings) { this.indexToSettings = indexToSettings; @@ -63,14 +59,25 @@ public GetSettingsResponse(ImmutableOpenMap indexToSettings, public ImmutableOpenMap getIndexToSettings() { return indexToSettings; } + public ImmutableOpenMap getIndexToDefaultSettings() { + return indexToDefaultSettings; + } + /** + * + * Returns the string value for the specified index and setting. If the includeDefaults + * flag was not set or set to false on the GetSettingsRequest, this method will only + * return a value where the setting was explicitly set on the index. If the includeDefaults + * flag was set to true on the GetSettingsRequest, this method will fall back to return the default + * value if the setting was not explicitly set. + */ public String getSetting(String index, String setting) { Settings settings = indexToSettings.get(index); - Settings defaultSettings = indexToDefaultSettings.get(index); if (setting != null) { - if (settings.hasValue(setting)) { + if (settings != null && settings.hasValue(setting)) { return settings.get(setting); } else { + Settings defaultSettings = indexToDefaultSettings.get(index); if (defaultSettings != null) { return defaultSettings.get(setting); } else { @@ -120,22 +127,10 @@ public void writeTo(StreamOutput out) throws IOException { } } - private static void validateSettingsFieldName(XContentParser parser) throws IOException { - String settingsFieldName = parser.currentName(); - XContentParserUtils.ensureExpectedToken(org.elasticsearch.common.xcontent.XContentParser.Token.FIELD_NAME, parser.currentToken(), - parser::getTokenLocation); - if (settingsFieldName.equals("settings") == false && settingsFieldName.equals("defaults") == false) { - String message = "Failed to parse object: expecting field with name [%s] but found [%s]"; - throw new org.elasticsearch.common.ParsingException(parser.getTokenLocation(), String.format(java.util.Locale.ROOT, message, - "(settings|defaults)", settingsFieldName)); - } - } - private static void parseSettingsField(XContentParser parser, String currentIndexName, Map indexToSettings, Map indexToDefaultSettings) throws IOException { XContentParserUtils.ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.currentToken(), parser::getTokenLocation); - validateSettingsFieldName(parser); - String settingsFieldName = parser.currentName(); //this will be either "settings" or "defaults" + String settingsFieldName = parser.currentName(); //this should be either "settings" or "defaults" parser.nextToken(); //go to settings object itself XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); @@ -147,9 +142,7 @@ private static void parseSettingsField(XContentParser parser, String currentInde indexToDefaultSettings.put(currentIndexName, Settings.fromXContent(parser)); break; default: - throw new IllegalStateException("Unknown field name for settings field! Should be \"settings\" or \"default\", but " + - "was: "+settingsFieldName); - + //We don't know this field, ignore it } } @@ -204,7 +197,7 @@ public String toString() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return toXContent(builder, params, true); + return toXContent(builder, params, indexToDefaultSettings.isEmpty()); } private XContentBuilder toXContent(XContentBuilder builder, Params params, boolean omitEmptySettings) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java index ab2bdbb22fe13..ac4c775daedd6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java @@ -73,6 +73,10 @@ protected GetSettingsResponse newResponse() { return new GetSettingsResponse(); } + private static boolean isFilteredRequest(GetSettingsRequest request) { + return CollectionUtils.isEmpty(request.names()) == false; + } + @Override protected void masterOperation(GetSettingsRequest request, ClusterState state, ActionListener listener) { Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, request); @@ -84,18 +88,21 @@ protected void masterOperation(GetSettingsRequest request, ClusterState state, A continue; } - Settings settings = settingsFilter.filter(indexMetaData.getSettings()); + Settings indexSettings = settingsFilter.filter(indexMetaData.getSettings()); if (request.humanReadable()) { - settings = IndexMetaData.addHumanReadableSettings(settings); + indexSettings = IndexMetaData.addHumanReadableSettings(indexSettings); } - if (CollectionUtils.isEmpty(request.names()) == false) { - settings = settings.filter(k -> Regex.simpleMatch(request.names(), k)); + if (isFilteredRequest(request)) { + indexSettings = indexSettings.filter(k -> Regex.simpleMatch(request.names(), k)); } - indexToSettingsBuilder.put(concreteIndex.getName(), settings); + indexToSettingsBuilder.put(concreteIndex.getName(), indexSettings); if (request.includeDefaults()) { - Settings defaultSettings = settingsFilter.filter(indexScopedSettings.diff(settings, this.settings)); + Settings defaultSettings = settingsFilter.filter(indexScopedSettings.diff(indexSettings, this.settings)); + if (isFilteredRequest(request)) { + defaultSettings = defaultSettings.filter(k -> Regex.simpleMatch(request.names(), k)); + } indexToDefaultSettingsBuilder.put(concreteIndex.getName(), defaultSettings); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequestTests.java new file mode 100644 index 0000000000000..e4c75beaf8270 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequestTests.java @@ -0,0 +1,67 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.indices.settings.get; + +import org.elasticsearch.Version; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.util.Base64; + +public class GetSettingsRequestTests extends ESTestCase { + private static final String TEST_622_REQUEST_BYTES = "ADwDAAEKdGVzdF9pbmRleA4BEHRlc3Rfc2V0dGluZ19rZXkB"; + private static final GetSettingsRequest TEST_622_REQUEST = new GetSettingsRequest() + .indices("test_index") + .names("test_setting_key") + .humanReadable(true); + private static final GetSettingsRequest TEST_700_REQUEST = new GetSettingsRequest() + .includeDefaults(true) + .humanReadable(true) + .indices("test_index") + .names("test_setting_key"); + + public void testSerdeRoundTrip() throws IOException { + BytesStreamOutput bso = new BytesStreamOutput(); + TEST_700_REQUEST.writeTo(bso); + + byte[] responseBytes = BytesReference.toBytes(bso.bytes()); + StreamInput si = StreamInput.wrap(responseBytes); + GetSettingsRequest deserialized = new GetSettingsRequest(si); + assertEquals(TEST_700_REQUEST, deserialized); + } + public void testSerializeBackwardsCompatibility() throws IOException { + BytesStreamOutput bso = new BytesStreamOutput(); + bso.setVersion(Version.V_6_2_2); + TEST_700_REQUEST.writeTo(bso); + + byte[] responseBytes = BytesReference.toBytes(bso.bytes()); + assertEquals(TEST_622_REQUEST_BYTES, Base64.getEncoder().encodeToString(responseBytes)); + } + public void testDeserializeBackwardsCompatibility() throws IOException { + StreamInput si = StreamInput.wrap(Base64.getDecoder().decode(TEST_622_REQUEST_BYTES)); + si.setVersion(Version.V_6_2_2); + GetSettingsRequest deserialized = new GetSettingsRequest(si); + assertEquals(TEST_622_REQUEST, deserialized); + } + +} diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java index e44eec7f812ee..b496cfb97f293 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java @@ -1,3 +1,22 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.elasticsearch.action.admin.indices.settings.get; import org.elasticsearch.common.settings.Settings; @@ -16,6 +35,18 @@ public void testIncludeDefaults() { GetSettingsResponse defaultsResponse = client().admin().indices().getSettings(defaultsRequest).actionGet(); assertNotNull("index.refresh_interval should be set as we are including defaults", defaultsResponse.getSetting(indexName, "index.refresh_interval")); - + } + public void testIncludeDefaultsWithFiltering() { + String indexName = "test_index"; + createIndex(indexName, Settings.EMPTY); + GetSettingsRequest defaultsRequest = new GetSettingsRequest().indices(indexName).includeDefaults(true) + .names("index.refresh_interval"); + GetSettingsResponse defaultsResponse = client().admin().indices().getSettings(defaultsRequest).actionGet(); + assertNotNull("index.refresh_interval should be set as we are including defaults", defaultsResponse.getSetting(indexName, + "index.refresh_interval")); + assertNull("index.number_of_shards should be null as this query is filtered", + defaultsResponse.getSetting(indexName, "index.number_of_shards")); + assertNull("index.warmer.enabled should be null as this query is filtered", + defaultsResponse.getSetting(indexName, "index.warmer.enabled")); } } From 39a1640940da1cfac47b3a3fe2d3f45412ff9f31 Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Mon, 16 Apr 2018 20:22:51 -0400 Subject: [PATCH 07/11] make parsing forward compatible --- .../org/elasticsearch/client/Request.java | 1 + .../elasticsearch/client/RequestTests.java | 23 ++++++- .../settings/get/GetSettingsResponse.java | 66 ++++++++++--------- .../get/GetSettingsResponseTests.java | 41 ++++-------- 4 files changed, 69 insertions(+), 62 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java index 7eddbf51d313a..e2a03e59c502a 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java @@ -602,6 +602,7 @@ static Request getSettings(GetSettingsRequest getSettingsRequest) throws IOExcep params.withIncludeDefaults(getSettingsRequest.includeDefaults()); params.withMasterTimeout(getSettingsRequest.masterNodeTimeout()); + String endpoint = endpoint(indices, "_settings", getSettingsRequest.names()); return new Request(HttpGet.METHOD_NAME, endpoint, params.getParams(), null); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index 848b0e8d4d217..afd7c8368eb8b 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -76,6 +76,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.lucene.uid.Versions; +import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -440,12 +441,32 @@ public void testGetSettings() throws IOException { setRandomLocal(getSettingsRequest, expectedParams); + if (randomBoolean()) { + //the request object will not have include_defaults present unless it is set to true + getSettingsRequest.includeDefaults(true); + expectedParams.put("include_defaults", Boolean.toString(true)); + } + Request request = Request.getSettings(getSettingsRequest); StringJoiner endpoint = new StringJoiner("/", "/", ""); if (indicesUnderTest != null && indicesUnderTest.length > 0) { endpoint.add(String.join(",", indicesUnderTest)); } endpoint.add("_settings"); + + if (randomBoolean()) { + String[] names = randomBoolean() ? null : new String[randomIntBetween(0, 3)]; + if (names != null) { + for (int x = 0; x < names.length; x++) { + names[x] = randomAlphaOfLengthBetween(3, 10); + } + } + getSettingsRequest.names(names); + if (names != null && names.length > 0) { + endpoint.add(String.join(",", names)); + } + } + assertThat(endpoint.toString(), equalTo(request.getEndpoint())); assertThat(request.getParameters(), equalTo(expectedParams)); assertThat(request.getMethod(), equalTo(HttpGet.METHOD_NAME)); @@ -580,7 +601,7 @@ public void testIndex() throws IOException { } public void testRefresh() { - String[] indices = randomIndicesNames(1, 5); + String[] indices = randomBoolean() ? null : randomIndicesNames(0, 5); RefreshRequest refreshRequest; if (randomBoolean()) { refreshRequest = new RefreshRequest(indices); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java index 5500129ee4fb6..554a9e4c99d7e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java @@ -37,6 +37,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.CharBuffer; +import java.util.Arrays; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -64,10 +65,9 @@ public ImmutableOpenMap getIndexToDefaultSettings() { } /** - * * Returns the string value for the specified index and setting. If the includeDefaults * flag was not set or set to false on the GetSettingsRequest, this method will only - * return a value where the setting was explicitly set on the index. If the includeDefaults + * return a value where the setting was explicitly set on the index. If the includeDefaults * flag was set to true on the GetSettingsRequest, this method will fall back to return the default * value if the setting was not explicitly set. */ @@ -129,23 +129,32 @@ public void writeTo(StreamOutput out) throws IOException { private static void parseSettingsField(XContentParser parser, String currentIndexName, Map indexToSettings, Map indexToDefaultSettings) throws IOException { - XContentParserUtils.ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.currentToken(), parser::getTokenLocation); - String settingsFieldName = parser.currentName(); //this should be either "settings" or "defaults" - parser.nextToken(); //go to settings object itself - XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); - switch (settingsFieldName) { - case "settings": - indexToSettings.put(currentIndexName, Settings.fromXContent(parser)); - break; - case "defaults": - indexToDefaultSettings.put(currentIndexName, Settings.fromXContent(parser)); - break; - default: - //We don't know this field, ignore it - } + if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + switch (parser.currentName()) { + case "settings": + indexToSettings.put(currentIndexName, Settings.fromXContent(parser)); + break; + case "defaults": + indexToDefaultSettings.put(currentIndexName, Settings.fromXContent(parser)); + break; + default: + parser.skipChildren(); + } + } else if (parser.currentToken() == XContentParser.Token.START_ARRAY) { + parser.skipChildren(); + } + parser.nextToken(); } + private static void parseIndexEntry(XContentParser parser, Map indexToSettings, + Map indexToDefaultSettings) throws IOException { + String indexName = parser.currentName(); + parser.nextToken(); + while (!parser.isClosed() && parser.currentToken() != XContentParser.Token.END_OBJECT) { + parseSettingsField(parser, indexName, indexToSettings, indexToDefaultSettings); + } + } public static GetSettingsResponse fromXContent(XContentParser parser) throws IOException { HashMap indexToSettings = new HashMap<>(); HashMap indexToDefaultSettings = new HashMap<>(); @@ -156,23 +165,15 @@ public static GetSettingsResponse fromXContent(XContentParser parser) throws IOE XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); parser.nextToken(); - while (!parser.isClosed() && parser.currentToken() == XContentParser.Token.FIELD_NAME) { - String indexName = parser.currentName(); - parser.nextToken(); //go to per-index settings object - XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation); - parser.nextToken(); - - parseSettingsField(parser, indexName, indexToSettings, indexToDefaultSettings); //this will get one of settings or defaults - parser.nextToken(); - - if (parser.currentToken() == XContentParser.Token.FIELD_NAME) { - //we have both settings and default settings, let's grab the second one - parseSettingsField(parser, indexName, indexToSettings, indexToDefaultSettings); + while (!parser.isClosed()) { + if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + //we must assume this is an index entry + parseIndexEntry(parser, indexToSettings, indexToDefaultSettings); + } else if (parser.currentToken() == XContentParser.Token.START_ARRAY) { + parser.skipChildren(); + } else { parser.nextToken(); } - - XContentParserUtils.ensureExpectedToken(XContentParser.Token.END_OBJECT, parser.currentToken(), parser::getTokenLocation); - parser.nextToken(); //we should now be positioned at the beginning of the next index's settings, or at the end } ImmutableOpenMap settingsMap = ImmutableOpenMap.builder().putAll(indexToSettings).build(); @@ -185,6 +186,7 @@ public static GetSettingsResponse fromXContent(XContentParser parser) throws IOE @Override public String toString() { + try { ByteArrayOutputStream baos = new ByteArrayOutputStream(); XContentBuilder builder = new XContentBuilder(JsonXContent.jsonXContent, baos); @@ -211,7 +213,7 @@ private XContentBuilder toXContent(XContentBuilder builder, Params params, boole builder.startObject("settings"); cursor.value.toXContent(builder, params); builder.endObject(); - if (!indexToDefaultSettings.isEmpty()) { + if (indexToDefaultSettings.isEmpty() == false) { builder.startObject("defaults"); indexToDefaultSettings.get(cursor.key).toXContent(builder, params); builder.endObject(); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java index 0e58a8724b197..6435c7fa2c804 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java @@ -37,6 +37,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Set; +import java.util.function.Predicate; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; @@ -44,16 +45,6 @@ public class GetSettingsResponseTests extends AbstractStreamableXContentTestCase { - private final Set indexNames; - public GetSettingsResponseTests() { - Set names = new HashSet(); - int numIndices = randomIntBetween(1, 5); - for (int x=0;x indexNames = new HashSet(); + int numIndices = randomIntBetween(1, 5); + for (int x=0;x getRandomFieldsExcludeFilter() { + //we do not want to add new fields at the root (index-level), or inside settings blocks + return f -> f.equals("") || f.contains(".settings") || f.contains(".defaults"); } private static GetSettingsResponse getExpectedTest622Response() { @@ -171,21 +171,4 @@ public void testTransportSerdeRoundTripCurrentVersion() throws IOException { Assert.assertEquals(responseWithExtraFields, newResponse); } - - public void testRoundtrip622DropsDefaults() throws IOException { - GetSettingsResponse responseWithExtraFields = getResponseWithNewFields(); - BytesStreamOutput bso = new BytesStreamOutput(); - bso.setVersion(Version.V_6_2_2); - responseWithExtraFields.writeTo(bso); - - byte[] responseBytes = BytesReference.toBytes(bso.bytes()); - StreamInput si = StreamInput.wrap(responseBytes); - si.setVersion(Version.V_6_2_2); - - GetSettingsResponse newResponse = new GetSettingsResponse(); - newResponse.readFrom(si); - - Assert.assertNotEquals(responseWithExtraFields, newResponse); - Assert.assertEquals(getExpectedTest622Response(), newResponse); - } } From 463202774c56f610cef9c3ed840baf538c5a0e13 Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Tue, 17 Apr 2018 13:07:12 -0400 Subject: [PATCH 08/11] address recent reviewer comments, including removing bad settings diff/filtering logic and adding a unit test for TransportGetSettingsAction --- .../org/elasticsearch/client/Request.java | 5 +- .../elasticsearch/client/RequestTests.java | 9 +- .../IndicesClientDocumentationIT.java | 4 +- .../settings/get/GetSettingsResponse.java | 14 ++ .../get/TransportGetSettingsAction.java | 2 +- ...ActionIT.java => GetSettingsActionIT.java} | 2 +- .../settings/get/GetSettingsActionTests.java | 146 ++++++++++++++++++ 7 files changed, 173 insertions(+), 9 deletions(-) rename server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/{TransportGetSettingsActionIT.java => GetSettingsActionIT.java} (97%) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java index e2a03e59c502a..444677a9b9f50 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java @@ -596,14 +596,15 @@ static Request rollover(RolloverRequest rolloverRequest) throws IOException { static Request getSettings(GetSettingsRequest getSettingsRequest) throws IOException { String[] indices = getSettingsRequest.indices() == null ? Strings.EMPTY_ARRAY : getSettingsRequest.indices(); + String[] names = getSettingsRequest.names() == null ? Strings.EMPTY_ARRAY : getSettingsRequest.names(); + Params params = Params.builder(); params.withIndicesOptions(getSettingsRequest.indicesOptions()); params.withLocal(getSettingsRequest.local()); params.withIncludeDefaults(getSettingsRequest.includeDefaults()); params.withMasterTimeout(getSettingsRequest.masterNodeTimeout()); - - String endpoint = endpoint(indices, "_settings", getSettingsRequest.names()); + String endpoint = endpoint(indices, "_settings", names); return new Request(HttpGet.METHOD_NAME, endpoint, params.getParams(), null); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index afd7c8368eb8b..751a70d22428d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -443,11 +443,12 @@ public void testGetSettings() throws IOException { if (randomBoolean()) { //the request object will not have include_defaults present unless it is set to true - getSettingsRequest.includeDefaults(true); - expectedParams.put("include_defaults", Boolean.toString(true)); + getSettingsRequest.includeDefaults(randomBoolean()); + if (getSettingsRequest.includeDefaults()) { + expectedParams.put("include_defaults", Boolean.toString(true)); + } } - Request request = Request.getSettings(getSettingsRequest); StringJoiner endpoint = new StringJoiner("/", "/", ""); if (indicesUnderTest != null && indicesUnderTest.length > 0) { endpoint.add(String.join(",", indicesUnderTest)); @@ -467,6 +468,8 @@ public void testGetSettings() throws IOException { } } + Request request = Request.getSettings(getSettingsRequest); + assertThat(endpoint.toString(), equalTo(request.getEndpoint())); assertThat(request.getParameters(), equalTo(expectedParams)); assertThat(request.getMethod(), equalTo(HttpGet.METHOD_NAME)); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java index 991004f1da876..eb9a3a5da46af 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java @@ -853,9 +853,9 @@ public void testGetSettingsWithDefaults() throws Exception { GetSettingsRequest request = new GetSettingsRequest().indices("index"); request.indicesOptions(IndicesOptions.lenientExpandOpen()); - // tag::get-settings-include-defaults + // tag::get-settings-request-include-defaults request.includeDefaults(true); // <1> - // end::get-settings-include-defaults + // end::get-settings-request-include-defaults GetSettingsResponse getSettingsResponse = client.indices().getSettings(request); String numberOfShardsString = getSettingsResponse.getSetting("index", "index.number_of_shards"); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java index 554a9e4c99d7e..60776a2df15db 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java @@ -57,9 +57,23 @@ public GetSettingsResponse(ImmutableOpenMap indexToSettings, GetSettingsResponse() { } + /** + * Returns a map of index name to {@link Settings} object. The returned {@link Settings} + * objects contain only those settings explicitly set on a given index. Any settings + * taking effect as defaults must be accessed via {@link #getIndexToDefaultSettings()}. + */ public ImmutableOpenMap getIndexToSettings() { return indexToSettings; } + + /** + * If the originating {@link GetSettingsRequest} object was configured to include + * defaults, this will contain a mapping of index name to {@link Settings} objects. + * The returned {@link Settings} objects will contain only those settings taking + * effect as defaults. Any settings explicitly set on the index will be available + * via {@link #getIndexToSettings()}. + * See also {@link GetSettingsRequest#includeDefaults(boolean)} + */ public ImmutableOpenMap getIndexToDefaultSettings() { return indexToDefaultSettings; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java index ac4c775daedd6..9ce3ab17d3310 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java @@ -99,7 +99,7 @@ protected void masterOperation(GetSettingsRequest request, ClusterState state, A indexToSettingsBuilder.put(concreteIndex.getName(), indexSettings); if (request.includeDefaults()) { - Settings defaultSettings = settingsFilter.filter(indexScopedSettings.diff(indexSettings, this.settings)); + Settings defaultSettings = settingsFilter.filter(indexScopedSettings.diff(indexSettings, Settings.EMPTY)); if (isFilteredRequest(request)) { defaultSettings = defaultSettings.filter(k -> Regex.simpleMatch(request.names(), k)); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionIT.java similarity index 97% rename from server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java rename to server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionIT.java index b496cfb97f293..9149d158e5f59 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsActionIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionIT.java @@ -22,7 +22,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESIntegTestCase; -public class TransportGetSettingsActionIT extends ESIntegTestCase { +public class GetSettingsActionIT extends ESIntegTestCase { public void testIncludeDefaults() { String indexName = "test_index"; createIndex(indexName, Settings.EMPTY); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java new file mode 100644 index 0000000000000..51fccf2da6bbd --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java @@ -0,0 +1,146 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.indices.settings.get; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.replication.ClusterStateCreationUtils; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.IndexScopedSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsFilter; +import org.elasticsearch.common.settings.SettingsModule; +import org.elasticsearch.index.Index; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.transport.CapturingTransport; +import org.elasticsearch.threadpool.TestThreadPool; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; +import org.junit.After; +import org.junit.Before; + +import java.util.Collections; +import java.util.concurrent.TimeUnit; + +import static org.elasticsearch.test.ClusterServiceUtils.createClusterService; + +public class GetSettingsActionTests extends ESTestCase { + + private TransportService transportService; + private ClusterService clusterService; + private ThreadPool threadPool; + private SettingsFilter settingsFilter; + private final String indexName = "test_index"; + + + private TestTransportGetSettingsAction getSettingsAction; + + class TestTransportGetSettingsAction extends TransportGetSettingsAction { + TestTransportGetSettingsAction() { + super(Settings.EMPTY, GetSettingsActionTests.this.transportService, GetSettingsActionTests.this.clusterService, + GetSettingsActionTests.this.threadPool, settingsFilter, new ActionFilters(Collections.emptySet()), + new Resolver(Settings.EMPTY), IndexScopedSettings.DEFAULT_SCOPED_SETTINGS); + } + @Override + protected void masterOperation(GetSettingsRequest request, ClusterState state, ActionListener listener) { + ClusterState stateWithIndex = ClusterStateCreationUtils.state(indexName, 1, 1); + super.masterOperation(request, stateWithIndex, listener); + } + } + @Before + public void setUp() throws Exception { + super.setUp(); + + settingsFilter = new SettingsModule(Settings.EMPTY, Collections.emptyList(), Collections.emptyList()).getSettingsFilter(); + threadPool = new TestThreadPool("GetSettingsActionTests"); + clusterService = createClusterService(threadPool); + CapturingTransport capturingTransport = new CapturingTransport(); + transportService = new TransportService(clusterService.getSettings(), capturingTransport, threadPool, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + boundAddress -> clusterService.localNode(), null, Collections.emptySet()); + transportService.start(); + transportService.acceptIncomingRequests(); + getSettingsAction = new GetSettingsActionTests.TestTransportGetSettingsAction(); + } + + @After + public void tearDown() throws Exception { + ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); + threadPool = null; + clusterService.close(); + super.tearDown(); + } + public void testIncludeDefaults() { + GetSettingsRequest noDefaultsRequest = new GetSettingsRequest().indices(indexName); + getSettingsAction.execute(null, noDefaultsRequest, ActionListener.wrap(noDefaultsResponse -> { + assertNull("index.refresh_interval should be null as it was never set", noDefaultsResponse.getSetting(indexName, + "index.refresh_interval")); + }, exception -> { + throw new AssertionError(exception); + })); + + GetSettingsRequest defaultsRequest = new GetSettingsRequest().indices(indexName).includeDefaults(true); + + getSettingsAction.execute(null, defaultsRequest, ActionListener.wrap(defaultsResponse -> { + assertNotNull("index.refresh_interval should be set as we are including defaults", defaultsResponse.getSetting(indexName, + "index.refresh_interval")); + }, exception -> { + throw new AssertionError(exception); + })); + + } + public void testIncludeDefaultsWithFiltering() { + GetSettingsRequest defaultsRequest = new GetSettingsRequest().indices(indexName).includeDefaults(true) + .names("index.refresh_interval"); + getSettingsAction.execute(null, defaultsRequest, ActionListener.wrap(defaultsResponse -> { + assertNotNull("index.refresh_interval should be set as we are including defaults", defaultsResponse.getSetting(indexName, + "index.refresh_interval")); + assertNull("index.number_of_shards should be null as this query is filtered", + defaultsResponse.getSetting(indexName, "index.number_of_shards")); + assertNull("index.warmer.enabled should be null as this query is filtered", + defaultsResponse.getSetting(indexName, "index.warmer.enabled")); + }, exception -> { + throw new AssertionError(exception); + })); + } + + static class Resolver extends IndexNameExpressionResolver { + Resolver(Settings settings) { + super(settings); + } + + @Override + public String[] concreteIndexNames(ClusterState state, IndicesRequest request) { + return request.indices(); + } + + @Override + public Index[] concreteIndices(ClusterState state, IndicesRequest request) { + Index[] out = new Index[request.indices().length]; + for (int x = 0; x < out.length; x++) { + out[x] = new Index(request.indices()[x], "_na_"); + } + return out; + } + } +} From f19c6988ef5f072868fe0eab62a2044001643006 Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Thu, 19 Apr 2018 16:14:55 -0400 Subject: [PATCH 09/11] remove extraneous test --- .../settings/get/GetSettingsResponseTests.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java index 6435c7fa2c804..cf125257c36a8 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponseTests.java @@ -157,18 +157,4 @@ public void testCanOutput622Response() throws IOException { Assert.assertEquals(TEST_6_2_2_RESPONSE_BYTES, base64OfResponse); } - - public void testTransportSerdeRoundTripCurrentVersion() throws IOException { - GetSettingsResponse responseWithExtraFields = getResponseWithNewFields(); - BytesStreamOutput bso = new BytesStreamOutput(); - responseWithExtraFields.writeTo(bso); - - byte[] responseBytes = BytesReference.toBytes(bso.bytes()); - StreamInput si = StreamInput.wrap(responseBytes); - - GetSettingsResponse newResponse = new GetSettingsResponse(); - newResponse.readFrom(si); - - Assert.assertEquals(responseWithExtraFields, newResponse); - } } From e2a592eafad9e13ed054d96713cef9f7b43e8dea Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Mon, 30 Apr 2018 10:54:08 -0400 Subject: [PATCH 10/11] Incorporate reviewer feedback --- .../IndicesClientDocumentationIT.java | 2 +- .../settings/get/GetSettingsRequest.java | 3 +- .../settings/get/GetSettingsResponse.java | 2 - .../settings/get/GetSettingsActionIT.java | 52 ------------------- .../settings/get/GetSettingsActionTests.java | 4 +- .../settings/get/GetSettingsRequestTests.java | 3 +- 6 files changed, 7 insertions(+), 59 deletions(-) delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionIT.java diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java index 1fb1f4d8ea9fc..8f19ab6c8aac9 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java @@ -788,7 +788,7 @@ public void testGetSettings() throws Exception { // tag::get-settings-request GetSettingsRequest request = new GetSettingsRequest().indices("index"); - // tag::get-settings-request + // end::get-settings-request // tag::get-settings-request-names request.names("index.number_of_shards"); // <1> diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java index 55c55ce72d886..0c4f63b71fbbb 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequest.java @@ -39,7 +39,7 @@ public class GetSettingsRequest extends MasterNodeReadRequest defaultSettingsMap = ImmutableOpenMap.builder().putAll(indexToDefaultSettings).build(); - return new GetSettingsResponse(settingsMap, defaultSettingsMap); } @Override public String toString() { - try { ByteArrayOutputStream baos = new ByteArrayOutputStream(); XContentBuilder builder = new XContentBuilder(JsonXContent.jsonXContent, baos); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionIT.java deleted file mode 100644 index 9149d158e5f59..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionIT.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.action.admin.indices.settings.get; - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.test.ESIntegTestCase; - -public class GetSettingsActionIT extends ESIntegTestCase { - public void testIncludeDefaults() { - String indexName = "test_index"; - createIndex(indexName, Settings.EMPTY); - GetSettingsRequest noDefaultsRequest = new GetSettingsRequest().indices(indexName); - GetSettingsResponse noDefaultsResponse = client().admin().indices().getSettings(noDefaultsRequest).actionGet(); - assertNull("index.refresh_interval should be null as it was never set", noDefaultsResponse.getSetting(indexName, - "index.refresh_interval")); - - GetSettingsRequest defaultsRequest = new GetSettingsRequest().indices(indexName).includeDefaults(true); - GetSettingsResponse defaultsResponse = client().admin().indices().getSettings(defaultsRequest).actionGet(); - assertNotNull("index.refresh_interval should be set as we are including defaults", defaultsResponse.getSetting(indexName, - "index.refresh_interval")); - } - public void testIncludeDefaultsWithFiltering() { - String indexName = "test_index"; - createIndex(indexName, Settings.EMPTY); - GetSettingsRequest defaultsRequest = new GetSettingsRequest().indices(indexName).includeDefaults(true) - .names("index.refresh_interval"); - GetSettingsResponse defaultsResponse = client().admin().indices().getSettings(defaultsRequest).actionGet(); - assertNotNull("index.refresh_interval should be set as we are including defaults", defaultsResponse.getSetting(indexName, - "index.refresh_interval")); - assertNull("index.number_of_shards should be null as this query is filtered", - defaultsResponse.getSetting(indexName, "index.number_of_shards")); - assertNull("index.warmer.enabled should be null as this query is filtered", - defaultsResponse.getSetting(indexName, "index.warmer.enabled")); - } -} diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java index 51fccf2da6bbd..11f0188c8c0b0 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java @@ -52,7 +52,6 @@ public class GetSettingsActionTests extends ESTestCase { private SettingsFilter settingsFilter; private final String indexName = "test_index"; - private TestTransportGetSettingsAction getSettingsAction; class TestTransportGetSettingsAction extends TransportGetSettingsAction { @@ -67,6 +66,7 @@ protected void masterOperation(GetSettingsRequest request, ClusterState state, A super.masterOperation(request, stateWithIndex, listener); } } + @Before public void setUp() throws Exception { super.setUp(); @@ -90,6 +90,7 @@ public void tearDown() throws Exception { clusterService.close(); super.tearDown(); } + public void testIncludeDefaults() { GetSettingsRequest noDefaultsRequest = new GetSettingsRequest().indices(indexName); getSettingsAction.execute(null, noDefaultsRequest, ActionListener.wrap(noDefaultsResponse -> { @@ -109,6 +110,7 @@ public void testIncludeDefaults() { })); } + public void testIncludeDefaultsWithFiltering() { GetSettingsRequest defaultsRequest = new GetSettingsRequest().indices(indexName).includeDefaults(true) .names("index.refresh_interval"); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequestTests.java index e4c75beaf8270..d70c77029917b 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsRequestTests.java @@ -49,6 +49,7 @@ public void testSerdeRoundTrip() throws IOException { GetSettingsRequest deserialized = new GetSettingsRequest(si); assertEquals(TEST_700_REQUEST, deserialized); } + public void testSerializeBackwardsCompatibility() throws IOException { BytesStreamOutput bso = new BytesStreamOutput(); bso.setVersion(Version.V_6_2_2); @@ -57,11 +58,11 @@ public void testSerializeBackwardsCompatibility() throws IOException { byte[] responseBytes = BytesReference.toBytes(bso.bytes()); assertEquals(TEST_622_REQUEST_BYTES, Base64.getEncoder().encodeToString(responseBytes)); } + public void testDeserializeBackwardsCompatibility() throws IOException { StreamInput si = StreamInput.wrap(Base64.getDecoder().decode(TEST_622_REQUEST_BYTES)); si.setVersion(Version.V_6_2_2); GetSettingsRequest deserialized = new GetSettingsRequest(si); assertEquals(TEST_622_REQUEST, deserialized); } - } From cfe8f2e9a9d5791d25a883d7b2666587511df522 Mon Sep 17 00:00:00 2001 From: Tom Callahan Date: Wed, 2 May 2018 10:54:26 -0400 Subject: [PATCH 11/11] fix item missed in merge --- .../java/org/elasticsearch/client/RequestConvertersTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index 7df64031da30f..1953c820b8af8 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -445,7 +445,7 @@ public void testGetSettings() throws IOException { } } - Request request = Request.getSettings(getSettingsRequest); + Request request = RequestConverters.getSettings(getSettingsRequest); assertThat(endpoint.toString(), equalTo(request.getEndpoint())); assertThat(request.getParameters(), equalTo(expectedParams));