Browse Source

Add support for flat_settings flag to all REST APIs that output settings

Closes #4140
Igor Motov 11 years ago
parent
commit
bec6527312

+ 35 - 0
docs/reference/api-conventions.asciidoc

@@ -67,6 +67,41 @@ being consumed by a monitoring tool, rather than intended for human
 consumption.  The default for the `human` flag is
 `false`. added[1.00.Beta,Previously defaulted to `true`]
 
+[float]
+=== Flat Settings
+
+The `flat_settings` flag affects rendering of the lists of settings. When
+flat_settings` flag is `true` settings are returned in a flat format:
+
+[source,js]
+--------------------------------------------------
+{
+  "persistent" : { },
+  "transient" : {
+    "discovery.zen.minimum_master_nodes" : "1"
+  }
+}
+--------------------------------------------------
+
+When the `flat_settings` flag is `false` settings are returned in a more
+human readable structured format:
+
+[source,js]
+--------------------------------------------------
+{
+  "persistent" : { },
+  "transient" : {
+    "discovery" : {
+      "zen" : {
+        "minimum_master_nodes" : "1"
+      }
+    }
+  }
+}
+--------------------------------------------------
+
+By default the `flat_settings` is set to `false`.
+
 [float]
 === Parameters
 

+ 5 - 2
rest-api-spec/api/cluster.get_settings.json

@@ -5,9 +5,12 @@
     "url": {
       "path": "/_cluster/settings",
       "paths": ["/_cluster/settings"],
-      "parts": {
-      },
+      "parts": {},
       "params": {
+        "flat_settings": {
+          "type": "boolean",
+          "description": "Return settings in flat format (default: false)"
+        }
       }
     },
     "body": null

+ 8 - 2
rest-api-spec/api/cluster.node_info.json

@@ -7,8 +7,14 @@
       "paths": ["/_nodes", "/_nodes/{node_id}", "/_nodes/settings", "/_nodes/{node_id}/settings", "/_nodes/os", "/_nodes/{node_id}/os", "/_nodes/process", "/_nodes/{node_id}/process", "/_nodes/jvm", "/_nodes/{node_id}/jvm", "/_nodes/thread_pool", "/_nodes/{node_id}/thread_pool", "/_nodes/network", "/_nodes/{node_id}/network", "/_nodes/transport", "/_nodes/{node_id}/transport", "/_nodes/http", "/_nodes/{node_id}/http", "/_nodes/plugin", "/_nodes/{node_id}/plugin"],
       "parts": {
         "node_id": {
-          "type" : "list",
-          "description" : "A comma-separated list of node IDs or names to limit the returned information; use `_local` to return information from the node you're connecting to, leave empty to get information from all nodes"
+          "type": "list",
+          "description": "A comma-separated list of node IDs or names to limit the returned information; use `_local` to return information from the node you're connecting to, leave empty to get information from all nodes"
+        },
+        "params": {
+          "flat_settings": {
+            "type": "boolean",
+            "description": "Return settings in flat format (default: false)"
+          }
         }
       }
     },

+ 6 - 3
rest-api-spec/api/cluster.put_settings.json

@@ -5,13 +5,16 @@
     "url": {
       "path": "/_cluster/settings",
       "paths": ["/_cluster/settings"],
-      "parts": {
-      },
+      "parts": {},
       "params": {
+        "flat_settings": {
+          "type": "boolean",
+          "description": "Return settings in flat format (default: false)"
+        }
       }
     },
     "body": {
-      "description" : "The settings to be updated. Can be either `transient` or `persistent` (survives cluster restart)."
+      "description": "The settings to be updated. Can be either `transient` or `persistent` (survives cluster restart)."
     }
   }
 }

+ 21 - 18
rest-api-spec/api/cluster.state.json

@@ -5,40 +5,43 @@
     "url": {
       "path": "/_cluster/state",
       "paths": ["/_cluster/state"],
-      "parts": {
-      },
+      "parts": {},
       "params": {
         "filter_blocks": {
-          "type" : "boolean",
-          "description" : "Do not return information about blocks"
+          "type": "boolean",
+          "description": "Do not return information about blocks"
         },
         "filter_index_templates": {
-          "type" : "boolean",
-          "description" : "Do not return information about index templates"
+          "type": "boolean",
+          "description": "Do not return information about index templates"
         },
         "filter_indices": {
-          "type" : "list",
-          "description" : "Limit returned metadata information to specific indices"
+          "type": "list",
+          "description": "Limit returned metadata information to specific indices"
         },
         "filter_metadata": {
-          "type" : "boolean",
-          "description" : "Do not return information about indices metadata"
+          "type": "boolean",
+          "description": "Do not return information about indices metadata"
         },
         "filter_nodes": {
-          "type" : "boolean",
-          "description" : "Do not return information about nodes"
+          "type": "boolean",
+          "description": "Do not return information about nodes"
         },
         "filter_routing_table": {
-          "type" : "boolean",
-          "description" : "Do not return information about shard allocation (`routing_table` and `routing_nodes`)"
+          "type": "boolean",
+          "description": "Do not return information about shard allocation (`routing_table` and `routing_nodes`)"
         },
         "local": {
-          "type" : "boolean",
-          "description" : "Return local information, do not retrieve the state from master node (default: false)"
+          "type": "boolean",
+          "description": "Return local information, do not retrieve the state from master node (default: false)"
         },
         "master_timeout": {
-          "type" : "time",
-          "description" : "Specify timeout for connection to master"
+          "type": "time",
+          "description": "Specify timeout for connection to master"
+        },
+        "flat_settings": {
+          "type": "boolean",
+          "description": "Return settings in flat format (default: false)"
         }
       }
     },

+ 4 - 0
rest-api-spec/api/indices.get_settings.json

@@ -29,6 +29,10 @@
         "prefix" : {
            "type" : "string",
            "description" : "The prefix all settings must have in order to be included"
+        },
+        "flat_settings": {
+          "type": "boolean",
+          "description": "Return settings in flat format (default: false)"
         }
       }
     },

+ 4 - 0
rest-api-spec/api/indices.get_template.json

@@ -13,6 +13,10 @@
         }
       },
       "params": {
+          "flat_settings": {
+              "type": "boolean",
+              "description": "Return settings in flat format (default: false)"
+          }
       }
     },
     "body": null

+ 5 - 1
rest-api-spec/api/indices.put_settings.json

@@ -29,7 +29,11 @@
           "options": ["open", "closed"],
           "default": "open",
           "description": "Whether to expand wildcard expression to concrete indices that are open, closed or both."
-        }
+        },
+          "flat_settings": {
+              "type": "boolean",
+              "description": "Return settings in flat format (default: false)"
+          }
       }
     },
     "body": {

+ 5 - 1
rest-api-spec/api/indices.put_template.json

@@ -24,7 +24,11 @@
         "master_timeout": {
           "type" : "time",
           "description" : "Specify timeout for connection to master"
-        }
+        },
+          "flat_settings": {
+              "type": "boolean",
+              "description": "Return settings in flat format (default: false)"
+          }
       }
     },
     "body": {

+ 3 - 1
rest-api-spec/test/cluster.put_settings/10_basic.yaml

@@ -5,10 +5,12 @@
         body:
           transient:
             discovery.zen.minimum_master_nodes: 1
+        flat_settings: true
 
   - match: {transient: {discovery.zen.minimum_master_nodes: "1"}}
 
   - do:
-      cluster.get_settings: {}
+      cluster.get_settings:
+        flat_settings: true
 
   - match: {transient: {discovery.zen.minimum_master_nodes: "1"}}

+ 4 - 2
rest-api-spec/test/indices.put_settings/10_basic.yaml

@@ -11,6 +11,7 @@
   - do:
       indices.get_settings:
         index: test-index
+        flat_settings: true
 
   - match:
       test-index.settings.index\.number_of_replicas: "0"
@@ -21,8 +22,9 @@
           number_of_replicas: 1
 
   - do:
-      indices.get_settings: {}
+      indices.get_settings:
+        flat_settings: false
 
   - match:
-      test-index.settings.index\.number_of_replicas: "1"
+      test-index.settings.index.number_of_replicas: "1"
 

+ 1 - 3
src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoResponse.java

@@ -105,9 +105,7 @@ public class NodesInfoResponse extends NodesOperationResponse<NodeInfo> implemen
             if (nodeInfo.getSettings() != null) {
                 builder.startObject("settings");
                 Settings settings = settingsFilter != null ? settingsFilter.filterSettings(nodeInfo.getSettings()) : nodeInfo.getSettings();
-                for (Map.Entry<String, String> entry : settings.getAsMap().entrySet()) {
-                    builder.field(entry.getKey(), entry.getValue(), XContentBuilder.FieldCaseConversion.NONE);
-                }
+                settings.toXContent(builder, params);
                 builder.endObject();
             }
 

+ 2 - 7
src/main/java/org/elasticsearch/cluster/ClusterState.java

@@ -301,10 +301,7 @@ public class ClusterState implements ToXContent {
                 if (settingsFilter != null) {
                     settings = settingsFilter.filterSettings(settings);
                 }
-
-                for (Map.Entry<String, String> entry : settings.getAsMap().entrySet()) {
-                    builder.field(entry.getKey(), entry.getValue());
-                }
+                settings.toXContent(builder, params);
                 builder.endObject();
 
                 builder.startObject("mappings");
@@ -337,9 +334,7 @@ public class ClusterState implements ToXContent {
                 if (settingsFilter != null) {
                     settings = settingsFilter.filterSettings(settings);
                 }
-                for (Map.Entry<String, String> entry : settings.getAsMap().entrySet()) {
-                    builder.field(entry.getKey(), entry.getValue());
-                }
+                settings.toXContent(builder, params);
                 builder.endObject();
 
                 builder.startObject("mappings");

+ 106 - 0
src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java

@@ -35,6 +35,7 @@ import org.elasticsearch.common.property.PropertyPlaceholder;
 import org.elasticsearch.common.settings.loader.SettingsLoader;
 import org.elasticsearch.common.settings.loader.SettingsLoaderFactory;
 import org.elasticsearch.common.unit.*;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -78,6 +79,97 @@ public class ImmutableSettings implements Settings {
         return this.settings;
     }
 
+    @Override
+    public Map<String, Object> getAsStructuredMap() {
+        Map<String, Object> map = Maps.newHashMapWithExpectedSize(2);
+        for (Map.Entry<String, String> entry : settings.entrySet()) {
+            processSetting(map, "", entry.getKey(), entry.getValue());
+        }
+        for (Map.Entry<String, Object> entry : map.entrySet()) {
+            if (entry.getValue() instanceof Map) {
+                @SuppressWarnings("unchecked") Map<String, Object> valMap = (Map<String, Object>) entry.getValue();
+                entry.setValue(convertMapsToArrays(valMap));
+            }
+        }
+
+        return map;
+    }
+
+    private void processSetting(Map<String, Object> map, String prefix, String setting, String value) {
+        int prefixLength = setting.indexOf('.');
+        if (prefixLength == -1) {
+            @SuppressWarnings("unchecked") Map<String, Object> innerMap = (Map<String, Object>) map.get(prefix + setting);
+            if (innerMap != null) {
+                // It supposed to be a value, but we already have a map stored, need to convert this map to "." notation
+                for (Map.Entry<String, Object> entry : innerMap.entrySet()) {
+                    map.put(prefix + setting + "." + entry.getKey(), entry.getValue());
+                }
+            }
+            map.put(prefix + setting, value);
+        } else {
+            String key = setting.substring(0, prefixLength);
+            String rest = setting.substring(prefixLength + 1);
+            Object existingValue = map.get(prefix + key);
+            if (existingValue == null) {
+                Map<String, Object> newMap = Maps.newHashMapWithExpectedSize(2);
+                processSetting(newMap, "", rest, value);
+                map.put(key, newMap);
+            } else {
+                if (existingValue instanceof Map) {
+                    @SuppressWarnings("unchecked")
+                    Map<String, Object> innerMap = (Map<String, Object>) existingValue;
+                    processSetting(innerMap, "", rest, value);
+                    map.put(key, innerMap);
+                } else {
+                    // It supposed to be a map, but we already have a value stored, which is not a map
+                    // fall back to "." notation
+                    processSetting(map, prefix + key + ".", rest, value);
+                }
+            }
+        }
+    }
+
+    private Object convertMapsToArrays(Map<String, Object> map) {
+        if (map.isEmpty()) {
+            return map;
+        }
+        boolean isArray = true;
+        int maxIndex = -1;
+        for (Map.Entry<String, Object> entry : map.entrySet()) {
+            if (isArray) {
+                try {
+                    int index = Integer.parseInt(entry.getKey());
+                    if (index >= 0) {
+                        maxIndex = Math.max(maxIndex, index);
+                    } else {
+                        isArray = false;
+                    }
+                } catch (NumberFormatException ex) {
+                    isArray = false;
+                }
+            }
+            if (entry.getValue() instanceof Map) {
+                @SuppressWarnings("unchecked") Map<String, Object> valMap = (Map<String, Object>) entry.getValue();
+                entry.setValue(convertMapsToArrays(valMap));
+            }
+        }
+        if (isArray && (maxIndex + 1) == map.size()) {
+            ArrayList<Object> newValue = Lists.newArrayListWithExpectedSize(maxIndex + 1);
+            for (int i = 0; i <= maxIndex; i++) {
+                Object obj = map.get(Integer.toString(i));
+                if (obj == null) {
+                    // Something went wrong. Different format?
+                    // Bailout!
+                    return map;
+                }
+                newValue.add(obj);
+            }
+            return newValue;
+        }
+        return map;
+    }
+
+
     @Override
     public Settings getComponentSettings(Class component) {
         if (component.getName().startsWith("org.elasticsearch")) {
@@ -500,6 +592,20 @@ public class ImmutableSettings implements Settings {
         return new Builder();
     }
 
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        if (!params.paramAsBoolean("flat_settings", false)) {
+            for (Map.Entry<String, Object> entry : getAsStructuredMap().entrySet()) {
+                builder.field(entry.getKey(), entry.getValue());
+            }
+        } else {
+            for (Map.Entry<String, String> entry : getAsMap().entrySet()) {
+                builder.field(entry.getKey(), entry.getValue(), XContentBuilder.FieldCaseConversion.NONE);
+            }
+        }
+        return builder;
+    }
+
     /**
      * A builder allowing to put different settings and then {@link #build()} an immutable
      * settings implementation. Use {@link ImmutableSettings#settingsBuilder()} in order to

+ 10 - 4
src/main/java/org/elasticsearch/common/settings/Settings.java

@@ -25,6 +25,7 @@ import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.common.unit.SizeValue;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.common.xcontent.ToXContent;
 
 import java.util.Map;
 
@@ -36,7 +37,7 @@ import java.util.Map;
  *
  * @see ImmutableSettings
  */
-public interface Settings {
+public interface Settings extends ToXContent {
 
     /**
      * Component settings for a specific component. Returns all the settings for the given class, where the
@@ -69,10 +70,15 @@ public interface Settings {
     ClassLoader getClassLoaderIfSet();
 
     /**
-     * The settings as a {@link java.util.Map}.
+     * The settings as a flat {@link java.util.Map}.
      */
     ImmutableMap<String, String> getAsMap();
 
+    /**
+     * The settings as a structured {@link java.util.Map}.
+     */
+    Map<String, Object> getAsStructuredMap();
+
     /**
      * Returns the setting value associated with the setting key.
      *
@@ -247,8 +253,8 @@ public interface Settings {
      * <p>It will also automatically load a comma separated list under the settingPrefix and merge with
      * the numbered format.
      *
-     * @param settingPrefix The setting prefix to load the array by
-     * @param defaultArray The default array to use if no value is specified
+     * @param settingPrefix  The setting prefix to load the array by
+     * @param defaultArray   The default array to use if no value is specified
      * @param commaDelimited Whether to try to parse a string as a comma-delimited value
      * @return The setting array values
      * @throws SettingsException

+ 2 - 7
src/main/java/org/elasticsearch/rest/action/admin/cluster/settings/RestClusterGetSettingsAction.java

@@ -31,7 +31,6 @@ import org.elasticsearch.rest.*;
 import org.elasticsearch.rest.action.support.RestXContentBuilder;
 
 import java.io.IOException;
-import java.util.Map;
 
 /**
  */
@@ -57,15 +56,11 @@ public class RestClusterGetSettingsAction extends BaseRestHandler {
                     builder.startObject();
 
                     builder.startObject("persistent");
-                    for (Map.Entry<String, String> entry : response.getState().metaData().persistentSettings().getAsMap().entrySet()) {
-                        builder.field(entry.getKey(), entry.getValue());
-                    }
+                    response.getState().metaData().persistentSettings().toXContent(builder, request);
                     builder.endObject();
 
                     builder.startObject("transient");
-                    for (Map.Entry<String, String> entry : response.getState().metaData().transientSettings().getAsMap().entrySet()) {
-                        builder.field(entry.getKey(), entry.getValue());
-                    }
+                    response.getState().metaData().transientSettings().toXContent(builder, request);
                     builder.endObject();
 
                     builder.endObject();

+ 2 - 6
src/main/java/org/elasticsearch/rest/action/admin/cluster/settings/RestClusterUpdateSettingsAction.java

@@ -70,15 +70,11 @@ public class RestClusterUpdateSettingsAction extends BaseRestHandler {
             @Override
             protected void addCustomFields(XContentBuilder builder, ClusterUpdateSettingsResponse response) throws IOException {
                 builder.startObject("persistent");
-                for (Map.Entry<String, String> entry : response.getPersistentSettings().getAsMap().entrySet()) {
-                    builder.field(entry.getKey(), entry.getValue());
-                }
+                response.getPersistentSettings().toXContent(builder, request);
                 builder.endObject();
 
                 builder.startObject("transient");
-                for (Map.Entry<String, String> entry : response.getTransientSettings().getAsMap().entrySet()) {
-                    builder.field(entry.getKey(), entry.getValue());
-                }
+                response.getTransientSettings().toXContent(builder, request);
                 builder.endObject();
             }
 

+ 2 - 5
src/main/java/org/elasticsearch/rest/action/admin/indices/settings/RestGetSettingsAction.java

@@ -34,7 +34,6 @@ import org.elasticsearch.rest.*;
 import org.elasticsearch.rest.action.support.RestXContentBuilder;
 
 import java.io.IOException;
-import java.util.Map;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
 import static org.elasticsearch.rest.RestStatus.NOT_FOUND;
@@ -66,11 +65,9 @@ public class RestGetSettingsAction extends BaseRestHandler {
                     builder.startObject();
                     for (ObjectObjectCursor<String, Settings> cursor : getSettingsResponse.getIndexToSettings()) {
                         builder.startObject(cursor.key, XContentBuilder.FieldCaseConversion.NONE);
+                        foundAny = true;
                         builder.startObject(Fields.SETTINGS);
-                        for (Map.Entry<String, String> entry : cursor.value.getAsMap().entrySet()) {
-                            foundAny = true;
-                            builder.field(entry.getKey(), entry.getValue());
-                        }
+                        cursor.value.toXContent(builder, request);
                         builder.endObject();
                         builder.endObject();
                     }

+ 56 - 1
src/test/java/org/elasticsearch/common/settings/ImmutableSettingsTests.java

@@ -22,14 +22,18 @@ package org.elasticsearch.common.settings;
 import org.elasticsearch.common.settings.bar.BarTestClass;
 import org.elasticsearch.common.settings.foo.FooTestClass;
 import org.elasticsearch.test.ElasticsearchTestCase;
+import org.hamcrest.Matchers;
 import org.junit.Test;
 
+import java.util.List;
+import java.util.Map;
+
 import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
 import static org.hamcrest.Matchers.*;
 
 /**
  */
-public class ImmutableSettingsTests extends ElasticsearchTestCase{
+public class ImmutableSettingsTests extends ElasticsearchTestCase {
 
     @Test
     public void testGetAsClass() {
@@ -120,4 +124,55 @@ public class ImmutableSettingsTests extends ElasticsearchTestCase{
                 .build();
         assertThat(settings.get("setting1"), is(nullValue()));
     }
+
+    @Test
+    public void testUnFlattenedSettings() {
+        Settings settings = settingsBuilder()
+                .put("foo", "abc")
+                .put("bar", "def")
+                .put("baz.foo", "ghi")
+                .put("baz.bar", "jkl")
+                .putArray("baz.arr", "a", "b", "c")
+                .build();
+        Map<String, Object> map = settings.getAsStructuredMap();
+        assertThat(map.keySet(), Matchers.<String>hasSize(3));
+        assertThat(map, allOf(
+                Matchers.<String, Object>hasEntry("foo", "abc"),
+                Matchers.<String, Object>hasEntry("bar", "def")));
+
+        @SuppressWarnings("unchecked") Map<String, Object> bazMap = (Map<String, Object>) map.get("baz");
+        assertThat(bazMap.keySet(), Matchers.<String>hasSize(3));
+        assertThat(bazMap, allOf(
+                Matchers.<String, Object>hasEntry("foo", "ghi"),
+                Matchers.<String, Object>hasEntry("bar", "jkl")));
+        @SuppressWarnings("unchecked") List<String> bazArr = (List<String>) bazMap.get("arr");
+        assertThat(bazArr, contains("a", "b", "c"));
+
+    }
+
+    @Test
+    public void testFallbackToFlattenedSettings() {
+        Settings settings = settingsBuilder()
+                .put("foo", "abc")
+                .put("foo.bar", "def")
+                .put("foo.baz", "ghi").build();
+        Map<String, Object> map = settings.getAsStructuredMap();
+        assertThat(map.keySet(), Matchers.<String>hasSize(3));
+        assertThat(map, allOf(
+                Matchers.<String, Object>hasEntry("foo", "abc"),
+                Matchers.<String, Object>hasEntry("foo.bar", "def"),
+                Matchers.<String, Object>hasEntry("foo.baz", "ghi")));
+
+        settings = settingsBuilder()
+                .put("foo.bar", "def")
+                .put("foo", "abc")
+                .put("foo.baz", "ghi")
+                .build();
+        map = settings.getAsStructuredMap();
+        assertThat(map.keySet(), Matchers.<String>hasSize(3));
+        assertThat(map, allOf(
+                Matchers.<String, Object>hasEntry("foo", "abc"),
+                Matchers.<String, Object>hasEntry("foo.bar", "def"),
+                Matchers.<String, Object>hasEntry("foo.baz", "ghi")));
+    }
 }