Allow affix settings to delegate to actual settings (#22523)

Affix settings are useful to namespace a certain setting. Yet, affix settings
must be specialized for their concrete type which causes lot of code duplication.
This commit allows to reuse an existing setting with and affix setting as soon as
a concrete key is available.

core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java

@@ -38,8 +38,8 @@ public final class ESLoggerFactory {
     public static final Setting<Level> LOG_DEFAULT_LEVEL_SETTING =
         new Setting<>("logger.level", Level.INFO.name(), Level::valueOf, Property.NodeScope);
     public static final Setting<Level> LOG_LEVEL_SETTING =
-        Setting.prefixKeySetting("logger.", Level.INFO.name(), Level::valueOf,
+        Setting.prefixKeySetting("logger.", (key) -> new Setting<>(key, Level.INFO.name(), Level::valueOf, Property.Dynamic,
-            Property.Dynamic, Property.NodeScope);
+            Property.NodeScope));
 
     public static Logger getLogger(String prefix, String name) {
         return getLogger(prefix, LogManager.getLogger(name));

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

@@ -42,12 +42,15 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.EnumSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
@@ -894,31 +897,22 @@ public class Setting<T> extends ToXContentToBytes {
      * can easily be added with this setting. Yet, prefix key settings don't support updaters out of the box unless
      * {@link #getConcreteSetting(String)} is used to pull the updater.
      */
-    public static <T> Setting<T> prefixKeySetting(String prefix, String defaultValue, Function<String, T> parser,
+    public static <T> Setting<T> prefixKeySetting(String prefix, Function<String, Setting<T>> delegateFactory) {
-                                                  Property... properties) {
+        return affixKeySetting(new AffixKey(prefix), delegateFactory);
-        return affixKeySetting(AffixKey.withPrefix(prefix), (s) -> defaultValue, parser, properties);
     }
 
     /**
      * This setting type allows to validate settings that have the same type and a common prefix and suffix. For instance
-     * storage.${backend}.enable=[true|false] can easily be added with this setting. Yet, adfix key settings don't support updaters
+     * storage.${backend}.enable=[true|false] can easily be added with this setting. Yet, affix key settings don't support updaters
      * out of the box unless {@link #getConcreteSetting(String)} is used to pull the updater.
      */
-    public static <T> Setting<T> affixKeySetting(String prefix, String suffix, Function<Settings, String> defaultValue,
+    public static <T> Setting<T> affixKeySetting(String prefix, String suffix, Function<String, Setting<T>> delegateFactory) {
-                                                 Function<String, T> parser, Property... properties) {
+        return affixKeySetting(new AffixKey(prefix, suffix), delegateFactory);
-        return affixKeySetting(AffixKey.withAffix(prefix, suffix), defaultValue, parser, properties);
     }
 
-    public static <T> Setting<T> affixKeySetting(String prefix, String suffix, String defaultValue, Function<String, T> parser,
+    private static <T> Setting<T> affixKeySetting(AffixKey key, Function<String, Setting<T>> delegateFactory) {
-                                                 Property... properties) {
+        Setting<T> delegate = delegateFactory.apply("_na_");
-        return affixKeySetting(prefix, suffix, (s) -> defaultValue, parser, properties);
+        return new Setting<T>(key, delegate.defaultValue, delegate.parser, delegate.properties.toArray(new Property[0])) {
-    }
-
-    public static <T> Setting<T> affixKeySetting(AffixKey key, Function<Settings, String> defaultValue, Function<String, T> parser,
-                                                 Property... properties) {
-        return new Setting<T>(key, defaultValue, parser, properties) {
-
-            @Override
             boolean isGroupSetting() {
                 return true;
             }
@@ -931,7 +925,7 @@ public class Setting<T> extends ToXContentToBytes {
             @Override
             public Setting<T> getConcreteSetting(String key) {
                 if (match(key)) {
-                    return new Setting<>(key, defaultValue, parser, properties);
+                    return delegateFactory.apply(key);
                 } else {
                     throw new IllegalArgumentException("key [" + key + "] must match [" + getKey() + "] but didn't.");
                 }
@@ -939,14 +933,19 @@ public class Setting<T> extends ToXContentToBytes {
 
             @Override
             public void diff(Settings.Builder builder, Settings source, Settings defaultSettings) {
-                for (Map.Entry<String, String> entry : defaultSettings.getAsMap().entrySet()) {
+                final Set<String> concreteSettings = new HashSet<>();
-                    if (match(entry.getKey())) {
+                for (String settingKey : defaultSettings.getAsMap().keySet()) {
-                        getConcreteSetting(entry.getKey()).diff(builder, source, defaultSettings);
+                    if (match(settingKey)) {
+                        concreteSettings.add(key.getConcreteString(settingKey));
                     }
                 }
+                for (String key : concreteSettings) {
+                    getConcreteSetting(key).diff(builder, source, defaultSettings);
+                }
             }
         };
-    }
+    };
+
 
 
     public interface Key {
@@ -1013,36 +1012,44 @@ public class Setting<T> extends ToXContentToBytes {
     }
 
     public static final class AffixKey implements Key {
-        public static AffixKey withPrefix(String prefix) {
+        private final Pattern pattern;
-            return new AffixKey(prefix, null);
-        }
-
-        public static AffixKey withAffix(String prefix, String suffix) {
-            return new AffixKey(prefix, suffix);
-        }
-
         private final String prefix;
         private final String suffix;
 
-        public AffixKey(String prefix, String suffix) {
+        AffixKey(String prefix) {
+            this(prefix, null);
+        }
+
+        AffixKey(String prefix, String suffix) {
             assert prefix != null || suffix != null: "Either prefix or suffix must be non-null";
+
             this.prefix = prefix;
             if (prefix.endsWith(".") == false) {
                 throw new IllegalArgumentException("prefix must end with a '.'");
             }
             this.suffix = suffix;
+            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+)?");
+            }
         }
 
         @Override
         public boolean match(String key) {
-            boolean match = true;
+            return pattern.matcher(key).matches();
-            if (prefix != null) {
+        }
-                match = key.startsWith(prefix);
+
+        /**
+         * Returns a string representation of the concrete setting key
+         */
+        String getConcreteString(String key) {
+            Matcher matcher = pattern.matcher(key);
+            if (matcher.matches() == false) {
+                throw new IllegalStateException("can't get concrete string for key " + key + " key doesn't match");
             }
-            if (suffix != null) {
+            return matcher.group(1);
-                match = match && key.endsWith(suffix);
-            }
-            return match;
         }
 
         public SimpleKey toConcreteKey(String missingPart) {

core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -214,7 +214,8 @@ public class ScopedSettingsTests extends ESTestCase {
         Setting<Integer> fooBarBaz = Setting.intSetting("foo.bar.baz", 1, Property.NodeScope);
         Setting<Integer> fooBar = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope);
         Setting<Settings> someGroup = Setting.groupSetting("some.group.", Property.Dynamic, Property.NodeScope);
-        Setting<Boolean> someAffix = Setting.affixKeySetting("some.prefix.", "somekey", "true", Boolean::parseBoolean, Property.NodeScope);
+        Setting<Boolean> someAffix = Setting.affixKeySetting("some.prefix.", "somekey", (key) -> Setting.boolSetting(key, true,
+            Property.NodeScope));
         Setting<List<String>> foorBarQuux =
                 Setting.listSetting("foo.bar.quux", Arrays.asList("a", "b", "c"), Function.identity(), Property.NodeScope);
         ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(fooBar, fooBarBaz, foorBarQuux,
@@ -253,6 +254,65 @@ public class ScopedSettingsTests extends ESTestCase {
         assertThat(diff.getAsInt("foo.bar", null), equalTo(1));
     }
 
+    public void testDiffWithAffixAndComplexMatcher() {
+        Setting<Integer> fooBarBaz = Setting.intSetting("foo.bar.baz", 1, Property.NodeScope);
+        Setting<Integer> fooBar = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope);
+        Setting<Settings> someGroup = Setting.groupSetting("some.group.", Property.Dynamic, Property.NodeScope);
+        Setting<Boolean> someAffix = Setting.affixKeySetting("some.prefix.", "somekey", (key) -> Setting.boolSetting(key, true,
+            Property.NodeScope));
+        Setting<List<String>> foorBarQuux = Setting.affixKeySetting("foo.", "quux",
+            (key) -> Setting.listSetting(key,  Arrays.asList("a", "b", "c"), Function.identity(), Property.NodeScope));
+        ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(fooBar, fooBarBaz, foorBarQuux,
+            someGroup, someAffix)));
+        Settings diff = settings.diff(Settings.builder().put("foo.bar", 5).build(), Settings.EMPTY);
+        assertEquals(1, diff.getAsMap().size());
+        assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(1));
+        assertNull(diff.getAsArray("foo.bar.quux", null)); // affix settings don't know their concrete keys
+
+        diff = settings.diff(
+            Settings.builder().put("foo.bar", 5).build(),
+            Settings.builder().put("foo.bar.baz", 17).putArray("foo.bar.quux", "d", "e", "f").build());
+        assertEquals(4, diff.getAsMap().size());
+        assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(17));
+        assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"d", "e", "f"});
+
+        diff = settings.diff(
+            Settings.builder().put("some.group.foo", 5).build(),
+            Settings.builder().put("some.group.foobar", 17, "some.group.foo", 25).build());
+        assertEquals(3, diff.getAsMap().size());
+        assertThat(diff.getAsInt("some.group.foobar", null), equalTo(17));
+        assertNull(diff.get("some.group.foo"));
+        assertNull(diff.getAsArray("foo.bar.quux", null)); // affix settings don't know their concrete keys
+        assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(1));
+        assertThat(diff.getAsInt("foo.bar", null), equalTo(1));
+
+        diff = settings.diff(
+            Settings.builder().put("some.prefix.foo.somekey", 5).build(),
+            Settings.builder().put("some.prefix.foobar.somekey", 17,
+                "some.prefix.foo.somekey", 18).build());
+        assertEquals(3, diff.getAsMap().size());
+        assertThat(diff.getAsInt("some.prefix.foobar.somekey", null), equalTo(17));
+        assertNull(diff.get("some.prefix.foo.somekey"));
+        assertNull(diff.getAsArray("foo.bar.quux", null)); // affix settings don't know their concrete keys
+        assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(1));
+        assertThat(diff.getAsInt("foo.bar", null), equalTo(1));
+
+        diff = settings.diff(
+            Settings.builder().put("some.prefix.foo.somekey", 5).build(),
+            Settings.builder().put("some.prefix.foobar.somekey", 17,
+                "some.prefix.foo.somekey", 18)
+            .putArray("foo.bar.quux", "x", "y", "z")
+            .putArray("foo.baz.quux", "d", "e", "f")
+                .build());
+        assertEquals(9, diff.getAsMap().size());
+        assertThat(diff.getAsInt("some.prefix.foobar.somekey", null), equalTo(17));
+        assertNull(diff.get("some.prefix.foo.somekey"));
+        assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"x", "y", "z"});
+        assertArrayEquals(diff.getAsArray("foo.baz.quux", null), new String[] {"d", "e", "f"});
+        assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(1));
+        assertThat(diff.getAsInt("foo.bar", null), equalTo(1));
+    }
+
     public void testUpdateTracer() {
         ClusterSettings settings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
         AtomicReference<List<String>> ref = new AtomicReference<>();

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

@@ -426,7 +426,7 @@ public class SettingTests extends ESTestCase {
     }
 
     public void testDynamicKeySetting() {
-        Setting<Boolean> setting = Setting.prefixKeySetting("foo.", "false", Boolean::parseBoolean, Property.NodeScope);
+        Setting<Boolean> setting = Setting.prefixKeySetting("foo.", (key) -> Setting.boolSetting(key, false, Property.NodeScope));
         assertTrue(setting.hasComplexMatcher());
         assertTrue(setting.match("foo.bar"));
         assertFalse(setting.match("foo"));
@@ -444,11 +444,11 @@ public class SettingTests extends ESTestCase {
 
     public void testAffixKeySetting() {
         Setting<Boolean> setting =
-            Setting.affixKeySetting("foo.", "enable", "false", Boolean::parseBoolean, Property.NodeScope);
+            Setting.affixKeySetting("foo.", "enable", (key) -> Setting.boolSetting(key, false, Property.NodeScope));
         assertTrue(setting.hasComplexMatcher());
         assertTrue(setting.match("foo.bar.enable"));
         assertTrue(setting.match("foo.baz.enable"));
-        assertTrue(setting.match("foo.bar.baz.enable"));
+        assertFalse(setting.match("foo.bar.baz.enable"));
         assertFalse(setting.match("foo.bar"));
         assertFalse(setting.match("foo.bar.baz.enabled"));
         assertFalse(setting.match("foo"));
@@ -459,11 +459,23 @@ public class SettingTests extends ESTestCase {
         IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> setting.getConcreteSetting("foo"));
         assertEquals("key [foo] must match [foo.*.enable] but didn't.", exc.getMessage());
 
-        exc = expectThrows(IllegalArgumentException.class, () -> Setting.affixKeySetting("foo", "enable", "false",
+        exc = expectThrows(IllegalArgumentException.class, () -> Setting.affixKeySetting("foo", "enable",
-            Boolean::parseBoolean, Property.NodeScope));
+            (key) -> Setting.boolSetting(key, false, Property.NodeScope)));
         assertEquals("prefix must end with a '.'", exc.getMessage());
+
+        Setting<List<String>> listAffixSetting = Setting.affixKeySetting("foo.", "bar",
+            (key) -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), Property.NodeScope));
+
+        assertTrue(listAffixSetting.hasComplexMatcher());
+        assertTrue(listAffixSetting.match("foo.test.bar"));
+        assertTrue(listAffixSetting.match("foo.test_1.bar"));
+        assertFalse(listAffixSetting.match("foo.buzz.baz.bar"));
+        assertFalse(listAffixSetting.match("foo.bar"));
+        assertFalse(listAffixSetting.match("foo.baz"));
+        assertFalse(listAffixSetting.match("foo"));
     }
 
+
     public void testMinMaxInt() {
         Setting<Integer> integerSetting = Setting.intSetting("foo.bar", 1, 0, 10, Property.NodeScope);
         try {
@@ -530,4 +542,5 @@ public class SettingTests extends ESTestCase {
         assertThat(setting.get(Settings.builder().put("foo", "12h").build()), equalTo(TimeValue.timeValueHours(12)));
         assertThat(setting.get(Settings.EMPTY).getMillis(), equalTo(random.getMillis() * factor));
     }
+
 }

plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java

@@ -25,6 +25,7 @@ import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.node.Node;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -34,19 +35,14 @@ import java.util.Map;
 import java.util.function.Function;
 
 public final class AzureStorageSettings {
-    private static final Setting.AffixKey TIMEOUT_KEY = Setting.AffixKey.withAffix(Storage.PREFIX, "timeout");
+    private static final Setting<TimeValue> TIMEOUT_SETTING = Setting.affixKeySetting(Storage.PREFIX, "timeout",
-
+        (key) -> Setting.timeSetting(key, Storage.TIMEOUT_SETTING, Setting.Property.NodeScope));
-    private static final Setting<TimeValue> TIMEOUT_SETTING = Setting.affixKeySetting(
-        TIMEOUT_KEY,
-        (s) -> Storage.TIMEOUT_SETTING.get(s).toString(),
-        (s) -> Setting.parseTimeValue(s, TimeValue.timeValueSeconds(-1), TIMEOUT_KEY.toString()),
-        Setting.Property.NodeScope);
     private static final Setting<String> ACCOUNT_SETTING =
-        Setting.affixKeySetting(Storage.PREFIX, "account", "", Function.identity(), Setting.Property.NodeScope);
+        Setting.affixKeySetting(Storage.PREFIX, "account", (key) -> Setting.simpleString(key, Setting.Property.NodeScope));
     private static final Setting<String> KEY_SETTING =
-        Setting.affixKeySetting(Storage.PREFIX, "key", "", Function.identity(), Setting.Property.NodeScope);
+        Setting.affixKeySetting(Storage.PREFIX, "key", (key) -> Setting.simpleString(key, Setting.Property.NodeScope));
     private static final Setting<Boolean> DEFAULT_SETTING =
-        Setting.affixKeySetting(Storage.PREFIX, "default", "false", Boolean::valueOf, Setting.Property.NodeScope);
+        Setting.affixKeySetting(Storage.PREFIX, "default", (key) -> Setting.boolSetting(key, false, Setting.Property.NodeScope));
 
 
     private final String name;