Settings: Introduce settings updater for a list of settings

server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -194,6 +194,16 @@ public synchronized <T> void addSettingsUpdateConsumer(Setting<T> setting, Consu
         addSettingsUpdater(setting.newUpdater(consumer, logger, validator));
     }
 
+    /**
+     * Adds a settings consumer that is only executed if any setting in the supplied list of settings is changed. In that case all the
+     * settings are specified in the argument are returned.
+     *
+     * Also automatically adds empty consumers for all settings in order to activate logging
+     */
+    public synchronized void addSettingsUpdateConsumer(Consumer<Settings> consumer, List<? extends Setting<?>> settings) {
+        addSettingsUpdater(Setting.groupedSettingsUpdater(consumer, logger, settings));
+    }
+
     /**
      * Adds a settings consumer for affix settings. Affix settings have a namespace associated to it that needs to be available to the
      * consumer in order to be processed correctly.

server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -509,10 +509,10 @@ public Tuple<A, B> getValue(Settings current, Settings previous) {
             @Override
             public void apply(Tuple<A, B> value, Settings current, Settings previous) {
                 if (aSettingUpdater.hasChanged(current, previous)) {
-                    logger.info("updating [{}] from [{}] to [{}]", aSetting.key, aSetting.getRaw(previous), aSetting.getRaw(current));
+                    logSettingUpdate(aSetting, current, previous, logger);
                 }
                 if (bSettingUpdater.hasChanged(current, previous)) {
-                    logger.info("updating [{}] from [{}] to [{}]", bSetting.key, bSetting.getRaw(previous), bSetting.getRaw(current));
+                    logSettingUpdate(bSetting, current, previous, logger);
                 }
                 consumer.accept(value.v1(), value.v2());
             }
@@ -524,6 +524,46 @@ public String toString() {
         };
     }
 
+    static AbstractScopedSettings.SettingUpdater<Settings> groupedSettingsUpdater(Consumer<Settings> consumer, Logger logger,
+                                                                                  final List<? extends Setting<?>> configuredSettings) {
+
+        return new AbstractScopedSettings.SettingUpdater<Settings>() {
+
+            private Settings get(Settings settings) {
+                return settings.filter(s -> {
+                    for (Setting<?> setting : configuredSettings) {
+                        if (setting.key.match(s)) {
+                            return true;
+                        }
+                    }
+                    return false;
+                });
+            }
+
+            @Override
+            public boolean hasChanged(Settings current, Settings previous) {
+                Settings currentSettings = get(current);
+                Settings previousSettings = get(previous);
+                return currentSettings.equals(previousSettings) == false;
+            }
+
+            @Override
+            public Settings getValue(Settings current, Settings previous) {
+                return get(current);
+            }
+
+            @Override
+            public void apply(Settings value, Settings current, Settings previous) {
+                consumer.accept(value);
+            }
+
+            @Override
+            public String toString() {
+                return "Updater grouped: " + configuredSettings.stream().map(Setting::getKey).collect(Collectors.joining(", "));
+            }
+        };
+    }
+
     public static class AffixSetting<T> extends Setting<T> {
         private final AffixKey key;
         private final Function<String, Setting<T>> delegateFactory;
@@ -541,7 +581,7 @@ boolean isGroupSetting() {
         }
 
         private Stream<String> matchStream(Settings settings) {
-            return settings.keySet().stream().filter((key) -> match(key)).map(settingKey -> key.getConcreteString(settingKey));
+            return settings.keySet().stream().filter(this::match).map(key::getConcreteString);
         }
 
         public Set<String> getSettingsDependencies(String settingsKey) {
@@ -812,9 +852,7 @@ public Settings getValue(Settings current, Settings previous) {
 
                 @Override
                 public void apply(Settings value, Settings current, Settings previous) {
-                    if (logger.isInfoEnabled()) { // getRaw can create quite some objects
-                        logger.info("updating [{}] from [{}] to [{}]", key, getRaw(previous), getRaw(current));
-                    }
+                    Setting.logSettingUpdate(GroupSetting.this, current, previous, logger);
                     consumer.accept(value);
                 }
 
@@ -902,7 +940,7 @@ public T getValue(Settings current, Settings previous) {
 
         @Override
         public void apply(T value, Settings current, Settings previous) {
-            logger.info("updating [{}] from [{}] to [{}]", key, getRaw(previous), getRaw(current));
+            logSettingUpdate(Setting.this, current, previous, logger);
             consumer.accept(value);
         }
     }
@@ -1138,6 +1176,16 @@ private static String arrayToParsableString(List<String> array) {
         }
     }
 
+    static void logSettingUpdate(Setting setting, Settings current, Settings previous, Logger logger) {
+        if (logger.isInfoEnabled()) {
+            if (setting.isFiltered()) {
+                logger.info("updating [{}]", setting.key);
+            } else {
+                logger.info("updating [{}] from [{}] to [{}]", setting.key, setting.getRaw(previous), setting.getRaw(current));
+            }
+        }
+    }
+
     public static Setting<Settings> groupSetting(String key, Property... properties) {
         return groupSetting(key, (s) -> {}, properties);
     }
@@ -1308,8 +1356,8 @@ public static final class AffixKey implements Key {
             if (suffix == null) {
                 pattern = Pattern.compile("(" + Pattern.quote(prefix) + "((?:[-\\w]+[.])*[-\\w]+$))");
             } else {
-                // the last part of this regexp is for lists since they are represented as x.${namespace}.y.1, x.${namespace}.y.2
-                pattern = Pattern.compile("(" + Pattern.quote(prefix) + "([-\\w]+)\\." + Pattern.quote(suffix) + ")(?:\\.\\d+)?");
+                // the last part of this regexp is to support both list and group keys
+                pattern = Pattern.compile("(" + Pattern.quote(prefix) + "([-\\w]+)\\." + Pattern.quote(suffix) + ")(?:\\..*)?");
             }
         }
 

server/src/test/java/org/elasticsearch/common/settings/SettingTests.java

@@ -38,6 +38,7 @@
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
@@ -712,4 +713,79 @@ public void testTimeValue() {
         assertThat(setting.get(Settings.EMPTY).getMillis(), equalTo(random.getMillis() * factor));
     }
 
+    public void testSettingsGroupUpdater() {
+        Setting<Integer> intSetting = Setting.intSetting("prefix.foo", 1, Property.NodeScope, Property.Dynamic);
+        Setting<Integer> intSetting2 = Setting.intSetting("prefix.same", 1, Property.NodeScope, Property.Dynamic);
+        AbstractScopedSettings.SettingUpdater<Settings> updater = Setting.groupedSettingsUpdater(s -> {}, logger,
+            Arrays.asList(intSetting, intSetting2));
+
+        Settings current = Settings.builder().put("prefix.foo", 123).put("prefix.same", 5555).build();
+        Settings previous = Settings.builder().put("prefix.foo", 321).put("prefix.same", 5555).build();
+        assertTrue(updater.apply(current, previous));
+    }
+
+    public void testSettingsGroupUpdaterRemoval() {
+        Setting<Integer> intSetting = Setting.intSetting("prefix.foo", 1, Property.NodeScope, Property.Dynamic);
+        Setting<Integer> intSetting2 = Setting.intSetting("prefix.same", 1, Property.NodeScope, Property.Dynamic);
+        AbstractScopedSettings.SettingUpdater<Settings> updater = Setting.groupedSettingsUpdater(s -> {}, logger,
+            Arrays.asList(intSetting, intSetting2));
+
+        Settings current = Settings.builder().put("prefix.same", 5555).build();
+        Settings previous = Settings.builder().put("prefix.foo", 321).put("prefix.same", 5555).build();
+        assertTrue(updater.apply(current, previous));
+    }
+
+    public void testSettingsGroupUpdaterWithAffixSetting() {
+        Setting<Integer> intSetting = Setting.intSetting("prefix.foo", 1, Property.NodeScope, Property.Dynamic);
+        Setting.AffixSetting<String> prefixKeySetting =
+            Setting.prefixKeySetting("prefix.foo.bar.", key -> Setting.simpleString(key, Property.NodeScope, Property.Dynamic));
+        Setting.AffixSetting<String> affixSetting =
+            Setting.affixKeySetting("prefix.foo.", "suffix", key -> Setting.simpleString(key,Property.NodeScope, Property.Dynamic));
+
+        AbstractScopedSettings.SettingUpdater<Settings> updater = Setting.groupedSettingsUpdater(s -> {}, logger,
+            Arrays.asList(intSetting, prefixKeySetting, affixSetting));
+
+        Settings.Builder currentSettingsBuilder = Settings.builder()
+            .put("prefix.foo.bar.baz", "foo")
+            .put("prefix.foo.infix.suffix", "foo");
+        Settings.Builder previousSettingsBuilder = Settings.builder()
+            .put("prefix.foo.bar.baz", "foo")
+            .put("prefix.foo.infix.suffix", "foo");
+        boolean removePrefixKeySetting = randomBoolean();
+        boolean changePrefixKeySetting = randomBoolean();
+        boolean removeAffixKeySetting = randomBoolean();
+        boolean changeAffixKeySetting = randomBoolean();
+        boolean removeAffixNamespace = randomBoolean();
+
+        if (removePrefixKeySetting) {
+            previousSettingsBuilder.remove("prefix.foo.bar.baz");
+        }
+        if (changePrefixKeySetting) {
+            currentSettingsBuilder.put("prefix.foo.bar.baz", "bar");
+        }
+        if (removeAffixKeySetting) {
+            previousSettingsBuilder.remove("prefix.foo.infix.suffix");
+        }
+        if (changeAffixKeySetting) {
+            currentSettingsBuilder.put("prefix.foo.infix.suffix", "bar");
+        }
+        if (removeAffixKeySetting == false && changeAffixKeySetting == false && removeAffixNamespace) {
+            currentSettingsBuilder.remove("prefix.foo.infix.suffix");
+            currentSettingsBuilder.put("prefix.foo.infix2.suffix", "bar");
+            previousSettingsBuilder.put("prefix.foo.infix2.suffix", "bar");
+        }
+
+        boolean expectedChange = removeAffixKeySetting || removePrefixKeySetting || changeAffixKeySetting || changePrefixKeySetting
+            || removeAffixNamespace;
+        assertThat(updater.apply(currentSettingsBuilder.build(), previousSettingsBuilder.build()), is(expectedChange));
+    }
+
+    public void testAffixNamespacesWithGroupSetting() {
+        final Setting.AffixSetting<Settings> affixSetting =
+            Setting.affixKeySetting("prefix.","suffix",
+                (key) -> Setting.groupSetting(key + ".", Setting.Property.Dynamic, Setting.Property.NodeScope));
+
+        assertThat(affixSetting.getNamespaces(Settings.builder().put("prefix.infix.suffix", "anything").build()), hasSize(1));
+        assertThat(affixSetting.getNamespaces(Settings.builder().put("prefix.infix.suffix.anything", "anything").build()), hasSize(1));
+    }
 }

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

@@ -18,16 +18,22 @@
  */
 package org.elasticsearch.common.settings;
 
-import org.elasticsearch.common.Strings;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.Logger;
+import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.common.logging.ServerLoggers;
+import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.MockLogAppender;
 import org.elasticsearch.test.rest.FakeRestRequest;
 
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.function.Consumer;
 
 import static org.hamcrest.CoreMatchers.equalTo;
 
@@ -100,7 +106,43 @@ public void testSettingsFiltering() throws IOException {
                 .build(),
             "a.b.*.d"
         );
+    }
+
+    public void testFilteredSettingIsNotLogged() throws Exception {
+        Settings oldSettings = Settings.builder().put("key", "old").build();
+        Settings newSettings = Settings.builder().put("key", "new").build();
+
+        Setting<String> filteredSetting = Setting.simpleString("key", Property.Filtered);
+        assertExpectedLogMessages((testLogger) -> Setting.logSettingUpdate(filteredSetting, newSettings, oldSettings, testLogger),
+            new MockLogAppender.SeenEventExpectation("secure logging", "org.elasticsearch.test", Level.INFO, "updating [key]"),
+            new MockLogAppender.UnseenEventExpectation("unwanted old setting name", "org.elasticsearch.test", Level.INFO, "*old*"),
+            new MockLogAppender.UnseenEventExpectation("unwanted new setting name", "org.elasticsearch.test", Level.INFO, "*new*")
+        );
+    }
+
+    public void testRegularSettingUpdateIsFullyLogged() throws Exception {
+        Settings oldSettings = Settings.builder().put("key", "old").build();
+        Settings newSettings = Settings.builder().put("key", "new").build();
+
+        Setting<String> regularSetting = Setting.simpleString("key");
+        assertExpectedLogMessages((testLogger) -> Setting.logSettingUpdate(regularSetting, newSettings, oldSettings, testLogger),
+            new MockLogAppender.SeenEventExpectation("regular logging", "org.elasticsearch.test", Level.INFO,
+            "updating [key] from [old] to [new]"));
+    }
 
+    private void assertExpectedLogMessages(Consumer<Logger> consumer,
+                                           MockLogAppender.LoggingExpectation ... expectations) throws IllegalAccessException {
+        Logger testLogger = Loggers.getLogger("org.elasticsearch.test");
+        MockLogAppender appender = new MockLogAppender();
+        ServerLoggers.addAppender(testLogger, appender);
+        try {
+            appender.start();
+            Arrays.stream(expectations).forEach(appender::addExpectation);
+            consumer.accept(testLogger);
+            appender.assertAllExpectationsMatched();
+        } finally {
+            ServerLoggers.removeAppender(testLogger, appender);
+        }
     }
 
     private void testFiltering(Settings source, Settings filtered, String... patterns) throws IOException {

test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java

@@ -92,7 +92,7 @@ public void match(LogEvent event) {
                         saw = true;
                     }
                 } else {
-                    if (event.getMessage().toString().contains(message)) {
+                    if (event.getMessage().getFormattedMessage().contains(message)) {
                         saw = true;
                     }
                 }