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.

5 files changed, 132 insertions, 56 deletions

diff --git a/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java b/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java
index 548c1da5a8..16f47f78dd 100644
--- a/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java
+++ b/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,
-            Property.Dynamic, Property.NodeScope);
+        Setting.prefixKeySetting("logger.", (key) -> new Setting<>(key, Level.INFO.name(), Level::valueOf, Property.Dynamic,
+            Property.NodeScope));
 
     public static Logger getLogger(String prefix, String name) {
         return getLogger(prefix, LogManager.getLogger(name));
diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java
index 4ed64b77c4..c4eb5a1121 100644
--- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java
+++ b/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,
-                                                  Property... properties) {
-        return affixKeySetting(AffixKey.withPrefix(prefix), (s) -> defaultValue, parser, properties);
+    public static <T> Setting<T> prefixKeySetting(String prefix, Function<String, Setting<T>> delegateFactory) {
+        return affixKeySetting(new AffixKey(prefix), delegateFactory);
     }
 
     /**
      * 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,
-                                                 Function<String, T> parser, Property... properties) {
-        return affixKeySetting(AffixKey.withAffix(prefix, suffix), defaultValue, parser, properties);
+    public static <T> Setting<T> affixKeySetting(String prefix, String suffix, Function<String, Setting<T>> delegateFactory) {
+        return affixKeySetting(new AffixKey(prefix, suffix), delegateFactory);
     }
 
-    public static <T> Setting<T> affixKeySetting(String prefix, String suffix, String defaultValue, Function<String, T> parser,
-                                                 Property... properties) {
-        return affixKeySetting(prefix, suffix, (s) -> defaultValue, parser, properties);
-    }
-
-    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
+    private static <T> Setting<T> affixKeySetting(AffixKey key, Function<String, Setting<T>> delegateFactory) {
+        Setting<T> delegate = delegateFactory.apply("_na_");
+        return new Setting<T>(key, delegate.defaultValue, delegate.parser, delegate.properties.toArray(new Property[0])) {
             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()) {
-                    if (match(entry.getKey())) {
-                        getConcreteSetting(entry.getKey()).diff(builder, source, defaultSettings);
+                final Set<String> concreteSettings = new HashSet<>();
+                for (String settingKey : defaultSettings.getAsMap().keySet()) {
+                    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) {
-            return new AffixKey(prefix, null);
-        }
-
-        public static AffixKey withAffix(String prefix, String suffix) {
-            return new AffixKey(prefix, suffix);
-        }
-
+        private final Pattern pattern;
         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;
-            if (prefix != null) {
-                match = key.startsWith(prefix);
-            }
-            if (suffix != null) {
-                match = match && key.endsWith(suffix);
+            return pattern.matcher(key).matches();
+        }
+
+        /**
+         * 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");
             }
-            return match;
+            return matcher.group(1);
         }
 
         public SimpleKey toConcreteKey(String missingPart) {
diff --git a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java
index 851ea26a19..1fbed949b5 100644
--- a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java
+++ b/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<>();
diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java
index 2bd5dea3c1..4ce23ebcaf 100644
--- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java
+++ b/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",
-            Boolean::parseBoolean, Property.NodeScope));
+        exc = expectThrows(IllegalArgumentException.class, () -> Setting.affixKeySetting("foo", "enable",
+            (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));
     }
+
 }
diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java b/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java
index 9d67eea628..5e0de46f65 100644
--- a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java
+++ b/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(
-        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<TimeValue> TIMEOUT_SETTING = Setting.affixKeySetting(Storage.PREFIX, "timeout",
+        (key) -> Setting.timeSetting(key, Storage.TIMEOUT_SETTING, 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;