Internal: refactor settings filtering

Refactor how settings filters are handled. Instead of specifying settings filters as filtering class, settings filters are now specified as list of settings that needs to be filtered out. Regex syntax is supported. This is breaking change and will require small change in plugins that are using settingsFilters. This change is needed in order to simplify cluster state diff implementation.

Contributes to #6295

src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoResponse.java

@@ -37,8 +37,6 @@ import java.util.Map;
  */
 public class NodesInfoResponse extends NodesOperationResponse<NodeInfo> implements ToXContent {
 
-    private SettingsFilter settingsFilter;
-
     public NodesInfoResponse() {
     }
 
@@ -64,11 +62,6 @@ public class NodesInfoResponse extends NodesOperationResponse<NodeInfo> implemen
         }
     }
 
-    public NodesInfoResponse settingsFilter(SettingsFilter settingsFilter) {
-        this.settingsFilter = settingsFilter;
-        return this;
-    }
-
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.field("cluster_name", getClusterName().value(), XContentBuilder.FieldCaseConversion.NONE);
@@ -102,7 +95,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();
+                Settings settings = nodeInfo.getSettings();
                 settings.toXContent(builder, params);
                 builder.endObject();
             }

src/main/java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java

@@ -86,7 +86,7 @@ public class TransportGetSettingsAction extends TransportMasterNodeReadOperation
                 continue;
             }
 
-            Settings settings = settingsFilter.filterSettings(indexMetaData.settings());
+            Settings settings = SettingsFilter.filterSettings(settingsFilter.getPatterns(), indexMetaData.settings());
             if (!CollectionUtils.isEmpty(request.names())) {
                 ImmutableSettings.Builder settingsBuilder = ImmutableSettings.builder();
                 for (Map.Entry<String, String> entry : settings.getAsMap().entrySet()) {

src/main/java/org/elasticsearch/cluster/ClusterState.java

@@ -33,7 +33,7 @@ import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.cluster.routing.*;
-import org.elasticsearch.cluster.routing.allocation.AllocationExplanation;
+
 import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Strings;
@@ -44,7 +44,6 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.settings.SettingsFilter;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -134,8 +133,6 @@ public class ClusterState implements ToXContent {
     // built on demand
     private volatile RoutingNodes routingNodes;
 
-    private SettingsFilter settingsFilter;
-
     private volatile ClusterStateStatus status;
 
     public ClusterState(long version, ClusterState state) {
@@ -234,11 +231,6 @@ public class ClusterState implements ToXContent {
         return routingNodes;
     }
 
-    public ClusterState settingsFilter(SettingsFilter settingsFilter) {
-        this.settingsFilter = settingsFilter;
-        return this;
-    }
-
     public String prettyPrint() {
         StringBuilder sb = new StringBuilder();
         sb.append("version: ").append(version).append("\n");
@@ -385,9 +377,6 @@ public class ClusterState implements ToXContent {
 
                 builder.startObject("settings");
                 Settings settings = templateMetaData.settings();
-                if (settingsFilter != null) {
-                    settings = settingsFilter.filterSettings(settings);
-                }
                 settings.toXContent(builder, params);
                 builder.endObject();
 
@@ -418,9 +407,6 @@ public class ClusterState implements ToXContent {
 
                 builder.startObject("settings");
                 Settings settings = indexMetaData.settings();
-                if (settingsFilter != null) {
-                    settings = settingsFilter.filterSettings(settings);
-                }
                 settings.toXContent(builder, params);
                 builder.endObject();
 

src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java

@@ -648,12 +648,13 @@ public class ImmutableSettings implements Settings {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        Settings settings = SettingsFilter.filterSettings(params, this);
         if (!params.paramAsBoolean("flat_settings", false)) {
-            for (Map.Entry<String, Object> entry : getAsStructuredMap().entrySet()) {
+            for (Map.Entry<String, Object> entry : settings.getAsStructuredMap().entrySet()) {
                 builder.field(entry.getKey(), entry.getValue());
             }
         } else {
-            for (Map.Entry<String, String> entry : getAsMap().entrySet()) {
+            for (Map.Entry<String, String> entry : settings.getAsMap().entrySet()) {
                 builder.field(entry.getKey(), entry.getValue(), XContentBuilder.FieldCaseConversion.NONE);
             }
         }

src/main/java/org/elasticsearch/common/settings/SettingsFilter.java

@@ -18,41 +18,95 @@
  */
 package org.elasticsearch.common.settings;
 
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.regex.Regex;
+import org.elasticsearch.common.xcontent.ToXContent.Params;
+import org.elasticsearch.rest.RestRequest;
 
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 import java.util.concurrent.CopyOnWriteArrayList;
 
+import static com.google.common.collect.Lists.newArrayList;
+
 /**
  *
  */
 public class SettingsFilter extends AbstractComponent {
+    /**
+     * Can be used to specify settings filter that will be used to filter out matching settings in toXContent method
+     */
+    public static String SETTINGS_FILTER_PARAM = "settings_filter";
 
-    public static interface Filter {
+    private final CopyOnWriteArrayList<String> patterns = new CopyOnWriteArrayList<>();
-
-        void filter(ImmutableSettings.Builder settings);
-    }
-
-    private final CopyOnWriteArrayList<Filter> filters = new CopyOnWriteArrayList<>();
 
     @Inject
     public SettingsFilter(Settings settings) {
         super(settings);
     }
 
-    public void addFilter(Filter filter) {
+    /**
-        filters.add(filter);
+     * Adds a new simple pattern to the list of filters
+     *
+     * @param pattern
+     */
+    public void addFilter(String pattern) {
+        patterns.add(pattern);
     }
 
-    public void removeFilter(Filter filter) {
+    /**
-        filters.remove(filter);
+     * Removes a simple pattern from the list of filters
+     *
+     * @param pattern
+     */
+    public void removeFilter(String pattern) {
+        patterns.remove(pattern);
     }
 
-    public Settings filterSettings(Settings settings) {
+    public String getPatterns() {
+        return Strings.collectionToDelimitedString(patterns, ",");
+    }
+
+    public void addFilterSettingParams(RestRequest request) {
+        if (patterns.isEmpty() == false) {
+            request.params().put(SETTINGS_FILTER_PARAM, getPatterns());
+        }
+    }
+
+    public static Settings filterSettings(Params params, Settings settings) {
+        String patterns = params.param(SETTINGS_FILTER_PARAM);
+        Settings filteredSettings = settings;
+        if (patterns != null && patterns.isEmpty() == false) {
+            filteredSettings = SettingsFilter.filterSettings(patterns, filteredSettings);
+        }
+        return filteredSettings;
+    }
+
+    public static Settings filterSettings(String patterns, Settings settings) {
+        String[] patternArray = Strings.delimitedListToStringArray(patterns, ",");
         ImmutableSettings.Builder builder = ImmutableSettings.settingsBuilder().put(settings);
-        for (Filter filter : filters) {
+        List<String> simpleMatchPatternList = newArrayList();
-            filter.filter(builder);
+        for (String pattern : patternArray) {
+            if (Regex.isSimpleMatchPattern(pattern)) {
+                simpleMatchPatternList.add(pattern);
+            } else {
+                builder.remove(pattern);
+            }
+        }
+        if (!simpleMatchPatternList.isEmpty()) {
+            String[] simpleMatchPatterns = simpleMatchPatternList.toArray(new String[simpleMatchPatternList.size()]);
+            Iterator<Entry<String, String>> iterator = builder.internalMap().entrySet().iterator();
+            while (iterator.hasNext()) {
+                Map.Entry<String, String> current = iterator.next();
+                if (Regex.simpleMatch(simpleMatchPatterns, current.getKey())) {
+                    iterator.remove();
+                }
+            }
         }
         return builder.build();
     }
-}
+}

src/main/java/org/elasticsearch/rest/action/admin/cluster/node/info/RestNodesInfoAction.java

@@ -99,11 +99,12 @@ public class RestNodesInfoAction extends BaseRestHandler {
             nodesInfoRequest.plugins(metrics.contains("plugins"));
         }
 
+        settingsFilter.addFilterSettingParams(request);
+
         client.admin().cluster().nodesInfo(nodesInfoRequest, new RestBuilderListener<NodesInfoResponse>(channel) {
 
             @Override
             public RestResponse buildResponse(NodesInfoResponse response, XContentBuilder builder) throws Exception {
-                response.settingsFilter(settingsFilter);
                 builder.startObject();
                 response.toXContent(builder, request);
                 builder.endObject();

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

@@ -71,7 +71,8 @@ public class RestClusterRerouteAction extends BaseRestHandler {
                 if (request.param("metric") == null) {
                     request.params().put("metric", DEFAULT_METRICS);
                 }
-                response.getState().settingsFilter(settingsFilter).toXContent(builder, request);
+                settingsFilter.addFilterSettingParams(request);
+                response.getState().toXContent(builder, request);
                 builder.endObject();
                 if (clusterRerouteRequest.explain()) {
                     assert response.getExplanations() != null;

src/main/java/org/elasticsearch/rest/action/admin/cluster/state/RestClusterStateAction.java

@@ -76,13 +76,14 @@ public class RestClusterStateAction extends BaseRestHandler {
             clusterStateRequest.metaData(metrics.contains(ClusterState.Metric.METADATA));
             clusterStateRequest.blocks(metrics.contains(ClusterState.Metric.BLOCKS));
         }
+        settingsFilter.addFilterSettingParams(request);
 
         client.admin().cluster().state(clusterStateRequest, new RestBuilderListener<ClusterStateResponse>(channel) {
             @Override
             public RestResponse buildResponse(ClusterStateResponse response, XContentBuilder builder) throws Exception {
                 builder.startObject();
                 builder.field(Fields.CLUSTER_NAME, response.getClusterName().value());
-                response.getState().settingsFilter(settingsFilter).toXContent(builder, request);
+                response.getState().toXContent(builder, request);
                 builder.endObject();
                 return new BytesRestResponse(RestStatus.OK, builder);
             }

src/test/java/org/elasticsearch/cluster/settings/SettingsFilteringTests.java

@@ -0,0 +1,115 @@
+/*
+ * 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.cluster.settings;
+
+import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse;
+import org.elasticsearch.common.inject.AbstractModule;
+import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.inject.Module;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.settings.SettingsFilter;
+import org.elasticsearch.plugins.AbstractPlugin;
+import org.elasticsearch.test.ElasticsearchIntegrationTest;
+import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
+import org.junit.Test;
+
+import java.util.Collection;
+
+import static com.google.common.collect.Lists.newArrayList;
+import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope.SUITE;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.nullValue;
+
+@ClusterScope(scope = SUITE, numDataNodes = 1)
+public class SettingsFilteringTests extends ElasticsearchIntegrationTest {
+
+    @Override
+    protected Settings nodeSettings(int nodeOrdinal) {
+        return ImmutableSettings.settingsBuilder()
+                .put(super.nodeSettings(nodeOrdinal))
+                .put("plugin.types", SettingsFilteringPlugin.class.getName())
+                .build();
+    }
+
+    public static class SettingsFilteringPlugin extends AbstractPlugin {
+        /**
+         * The name of the plugin.
+         */
+        @Override
+        public String name() {
+            return "settings-filtering";
+        }
+
+        /**
+         * The description of the plugin.
+         */
+        @Override
+        public String description() {
+            return "Settings Filtering Plugin";
+        }
+
+        @Override
+        public Collection<Class<? extends Module>> indexModules() {
+            Collection<Class<? extends Module>> modules = newArrayList();
+            modules.add(SettingsFilteringModule.class);
+            return modules;
+        }
+    }
+
+    public static class SettingsFilteringModule extends AbstractModule {
+
+        @Override
+        protected void configure() {
+            bind(SettingsFilteringService.class).asEagerSingleton();
+        }
+    }
+
+    public static class SettingsFilteringService {
+        @Inject
+        public SettingsFilteringService(SettingsFilter settingsFilter) {
+            settingsFilter.addFilter("index.filter_test.foo");
+            settingsFilter.addFilter("index.filter_test.bar*");
+        }
+    }
+
+
+    @Test
+    public void testSettingsFiltering() {
+
+        assertAcked(client().admin().indices().prepareCreate("test-idx").setSettings(ImmutableSettings.builder()
+                .put("filter_test.foo", "test")
+                .put("filter_test.bar1", "test")
+                .put("filter_test.bar2", "test")
+                .put("filter_test.notbar", "test")
+                .put("filter_test.notfoo", "test")
+                .build()).get());
+        GetSettingsResponse response = client().admin().indices().prepareGetSettings("test-idx").get();
+        Settings settings = response.getIndexToSettings().get("test-idx");
+
+        assertThat(settings.get("index.filter_test.foo"), nullValue());
+        assertThat(settings.get("index.filter_test.bar1"), nullValue());
+        assertThat(settings.get("index.filter_test.bar2"), nullValue());
+        assertThat(settings.get("index.filter_test.notbar"), equalTo("test"));
+        assertThat(settings.get("index.filter_test.notfoo"), equalTo("test"));
+    }
+
+}

src/test/java/org/elasticsearch/common/settings/SettingsFilterTests.java

@@ -0,0 +1,128 @@
+/*
+ * 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.common.settings;
+
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.rest.FakeRestRequest;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.test.ElasticsearchTestCase;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+
+public class SettingsFilterTests extends ElasticsearchTestCase {
+
+    @Test
+    public void testAddingAndRemovingFilters() {
+        SettingsFilter settingsFilter = new SettingsFilter(ImmutableSettings.EMPTY);
+        settingsFilter.addFilter("foo");
+        settingsFilter.addFilter("bar");
+        settingsFilter.addFilter("baz");
+        assertThat(settingsFilter.getPatterns(), equalTo("foo,bar,baz"));
+
+        settingsFilter.removeFilter("bar");
+        assertThat(settingsFilter.getPatterns(), equalTo("foo,baz"));
+
+        settingsFilter.removeFilter("bar");
+        settingsFilter.removeFilter("foo");
+        settingsFilter.removeFilter("baz");
+
+        assertThat(settingsFilter.getPatterns(), equalTo(""));
+    }
+
+    @Test
+    public void testSettingsFiltering() throws IOException {
+
+        testFiltering(ImmutableSettings.builder()
+                        .put("foo", "foo_test")
+                        .put("foo1", "foo1_test")
+                        .put("bar", "bar_test")
+                        .put("bar1", "bar1_test")
+                        .put("bar.2", "bar2_test")
+                        .build(),
+                ImmutableSettings.builder()
+                        .put("foo1", "foo1_test")
+                        .build(),
+                "foo,bar*"
+        );
+
+        testFiltering(ImmutableSettings.builder()
+                        .put("foo", "foo_test")
+                        .put("foo1", "foo1_test")
+                        .put("bar", "bar_test")
+                        .put("bar1", "bar1_test")
+                        .put("bar.2", "bar2_test")
+                        .build(),
+                ImmutableSettings.builder()
+                        .put("foo", "foo_test")
+                        .put("foo1", "foo1_test")
+                        .build(),
+                "bar*"
+        );
+
+        testFiltering(ImmutableSettings.builder()
+                        .put("foo", "foo_test")
+                        .put("foo1", "foo1_test")
+                        .put("bar", "bar_test")
+                        .put("bar1", "bar1_test")
+                        .put("bar.2", "bar2_test")
+                        .build(),
+                ImmutableSettings.builder()
+                        .build(),
+                "foo,bar*,foo*"
+        );
+
+        testFiltering(ImmutableSettings.builder()
+                        .put("foo", "foo_test")
+                        .put("bar", "bar_test")
+                        .put("baz", "baz_test")
+                        .build(),
+                ImmutableSettings.builder()
+                        .put("foo", "foo_test")
+                        .put("bar", "bar_test")
+                        .put("baz", "baz_test")
+                        .build()
+        );
+    }
+
+    private void testFiltering(Settings source, Settings filtered, String... patterns) throws IOException {
+        SettingsFilter settingsFilter = new SettingsFilter(ImmutableSettings.EMPTY);
+        for (String pattern : patterns) {
+            settingsFilter.addFilter(pattern);
+        }
+
+        // Test using direct filtering
+        Settings filteredSettings = SettingsFilter.filterSettings(settingsFilter.getPatterns(), source);
+        assertThat(filteredSettings.getAsMap().entrySet(), equalTo(filtered.getAsMap().entrySet()));
+
+        // Test using toXContent filtering
+        RestRequest request = new FakeRestRequest();
+        settingsFilter.addFilterSettingParams(request);
+        XContentBuilder xContentBuilder = XContentBuilder.builder(JsonXContent.jsonXContent);
+        xContentBuilder.startObject();
+        source.toXContent(xContentBuilder, request);
+        xContentBuilder.endObject();
+        String filteredSettingsString = xContentBuilder.string();
+        filteredSettings = ImmutableSettings.builder().loadFromSource(filteredSettingsString).build();
+        assertThat(filteredSettings.getAsMap().entrySet(), equalTo(filtered.getAsMap().entrySet()));
+    }
+}

src/test/java/org/elasticsearch/rest/FakeRestRequest.java

@@ -24,19 +24,22 @@ import org.elasticsearch.common.bytes.BytesReference;
 import java.util.HashMap;
 import java.util.Map;
 
-class FakeRestRequest extends RestRequest {
+public class FakeRestRequest extends RestRequest {
 
     private final Map<String, String> headers;
 
-    FakeRestRequest() {
+    private final Map<String, String> params;
+
+    public FakeRestRequest() {
         this(new HashMap<String, String>(), new HashMap<String, String>());
     }
 
-    FakeRestRequest(Map<String, String> headers, Map<String, String> context) {
+    public FakeRestRequest(Map<String, String> headers, Map<String, String> context) {
         this.headers = headers;
         for (Map.Entry<String, String> entry : context.entrySet()) {
             putInContext(entry.getKey(), entry.getValue());
         }
+        this.params = new HashMap<>();
     }
 
     @Override
@@ -81,21 +84,25 @@ class FakeRestRequest extends RestRequest {
 
     @Override
     public boolean hasParam(String key) {
-        return false;
+        return params.containsKey(key);
     }
 
     @Override
     public String param(String key) {
-        return null;
+        return params.get(key);
     }
 
     @Override
     public String param(String key, String defaultValue) {
-        return null;
+        String value = params.get(key);
+        if (value == null) {
+            return defaultValue;
+        }
+        return value;
     }
 
     @Override
     public Map<String, String> params() {
-        return null;
+        return params;
     }
 }